horribly wrong code when --with-versioned is active

* horribly wrong code when --with-versioned is active
@ 2013-08-05 21:46 John Spencer
  2013-08-06  5:57 ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: John Spencer @ 2013-08-05 21:46 UTC (permalink / raw)
  To: alsa-devel

if --with-versioned is active (default), a couple of macros in pcm.c
start generating some completely broken, __old-prefixed wrapper 
functions, which then are getting used whenever the actual function is 
called.

for example:
snd_pcm_hw_params_set_buffer_time_near

__OLD_NEAR1(snd_pcm_hw_params_set_buffer_time_near, unsigned int);

->

#define __OLD_NEAR1(name, ret_type) __P_OLD_NEAR1(__old_, name, ret_type)

->

#define __P_OLD_NEAR1(pfx, name, ret_type) \
ret_type pfx##name(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, ret_type 
val, int *dir) \
{ \
         if (INTERNAL(name)(pcm, params, &val, dir) < 0) \
                 return 0; \
         return (ret_type)val; \
}

this will lead to generating a function 
__old_snd_pcm_hw_params_set_buffer_time_near which expands to

unsigned int __old_snd_pcm_hw_params_set_buffer_time_near(snd_pcm_t 
*pcm, snd_pcm_hw_params_t *params, ret_type val, int *dir)
{
         if snd1_pcm_hw_params_set_buffer_time_near(pcm, params, &val, 
dir) < 0)
                 return 0;
         return (ret_type)val;
}

there 2 bugs in there,
1) the real function gets passed a pointer to a pointer of unsigned, 
which is then happily dereferenced and the original pointer used as an 
int, and
2) the return type logic is wrong, in case of a non-error, the original 
pointer will be returned instead of 0 to indicate success.

the right fix would look something like this:

@@ -7190,19 +7192,15 @@
  __OLD_GET1(snd_pcm_hw_params_get_tick_time_max, unsigned int, unsigned 
int);

  #define __P_OLD_NEAR(pfx, name, ret_type) \
-ret_type pfx##name(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, 
ret_type val) \
+ret_type pfx##name(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, 
ret_type *val) \
  { \
-	if (INTERNAL(name)(pcm, params, &val) < 0) \
-		return 0; \
-	return (ret_type)val; \
+	return(INTERNAL(name)(pcm, params, val)); \
  }

  #define __P_OLD_NEAR1(pfx, name, ret_type) \
-ret_type pfx##name(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, 
ret_type val, int *dir) \
+ret_type pfx##name(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, 
ret_type *val, int *dir) \
  { \
-	if (INTERNAL(name)(pcm, params, &val, dir) < 0) \
-		return 0; \
-	return (ret_type)val; \
+	return (INTERNAL(name)(pcm, params, val, dir) < 0); \
  }

  #define __OLD_NEAR(name, ret_type) __P_OLD_NEAR(__old_, name, ret_type)


this is only half of the fix though, the "old" getter functions seem to 
misbehave as well. the misbehaviour can be inspected by using a small 
openal-soft (version 1.14) example code, and breaking on 
alsa_reset_playback and single stepping through the invocation of 
CHECK(snd_pcm_hw_params_set_buffer_time_near(data->pcmHandle, hp, 
&bufferLen, NULL));

^ permalink raw reply	[flat|nested] 4+ messages in thread