Uninitialized variable in pa_linux_alsa.c

[PortAudio-admin]

Issue created by svesa on Assembla

The variable "dir" is used uninitialized on lines 923 and 933 (both have the same content):

ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, &dir ), paUnanticipatedHostError );

In other calls to "alsa_snd_pcm_hw_params_set_period_size_near" there is the following line just before each call:

dir = 0;

[PortAudio-admin]

Comment by @philburk

According to this doc: http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m___h_w___params.html#ga9162045265f283c532634506456cab09 dir is a direction and should be (-1,0,1). It is not clear from the documentation whether the function expects it to be set on entry. But it does seem safer to set it to zero rather than leaving it uninitialized.

[PortAudio-admin]

Comment by @RossBencina

Georgi D. Sotirov provided the following feedback via the mailing list:

https://lists.columbia.edu/pipermail/portaudio/2019-May/001812.html

I took some time today looking at PortAudio sources (which I'm building for Slackware Linux since more than 10 years BTW) in the Git repo as well as ALSA Library sources on GitHub. In PortAudio alsa_snd_pcm_hw_params_set_period_size_near is just a wrapper over snd_pcm_hw_params_set_period_size_near of ALSA, which looking at the sources in turn calls snd_pcm_hw_param_set_near. And this function uses the value of parameter dir to calculate the values of local variables valdir, mindir and maxdir (former two are also passed to other functions). So the variable dir in the call to snd_pcm_hw_params_set_period_size_near (a.k.a. alsa_snd_pcm_hw_params_set_period_size_near) should really be initialized or left NULL, so valdir is initialized with zero. Anyway, leaving the local variable uninitialized is not good, so a simple patch would be just to initialize it with zero, which seems to be the default (or just pass NULL).

diff --git a/src/hostapi/alsa/pa_linux_alsa.c b/src/hostapi/alsa/pa_linux_alsa.c
index 584cde8..3fac311 100644
--- a/src/hostapi/alsa/pa_linux_alsa.c
+++ b/src/hostapi/alsa/pa_linux_alsa.c
@@ -842,7 +842,7 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in
     double * defaultLowLatency, * defaultHighLatency, * defaultSampleRate =
         &devInfo->baseDeviceInfo.defaultSampleRate;
     double defaultSr = *defaultSampleRate;
-    int dir;
+    int dir = 0;

If someone could check more thoroughly what was the intention for the call of this ALSA function, this could eventually leave to a better patch. I see from the history in Git, that gineera committed this call in revision 28f9ee09658dc79ab186b7b5a7577ad177f43866 on 2013-06-08 also introducing the local variable, so perhaps he/she is the right person to review this part of the source and decide the best way to fix it.

[PortAudio-admin]

Comment by @RossBencina

The discussion has moved on, the preferred patch is now this one:

https://lists.columbia.edu/pipermail/portaudio/2019-May/001820.html

diff --git a/src/hostapi/alsa/pa_linux_alsa.c b/src/hostapi/alsa/pa_linux_alsa.c
index 584cde8..b5bdd3a 100644
--- a/src/hostapi/alsa/pa_linux_alsa.c
+++ b/src/hostapi/alsa/pa_linux_alsa.c
@@ -842,7 +842,6 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in
     double * defaultLowLatency, * defaultHighLatency, * defaultSampleRate =
         &devInfo->baseDeviceInfo.defaultSampleRate;
     double defaultSr = *defaultSampleRate;
-    int dir;

     assert( pcm );

@@ -920,7 +919,7 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in
     alsaBufferFrames = 512;
     alsaPeriodFrames = 128;
     ENSURE_( alsa_snd_pcm_hw_params_set_buffer_size_near( pcm, hwParams, &alsaBufferFrames ), paUnanticipatedHostError );
-    ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, &dir ), paUnanticipatedHostError );
+    ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, NULL ), paUnanticipatedHostError );
     *defaultLowLatency = (double) (alsaBufferFrames - alsaPeriodFrames) / defaultSr;

     /* Base the high latency case on values four times larger */
@@ -930,7 +929,7 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in
     ENSURE_( alsa_snd_pcm_hw_params_any( pcm, hwParams ), paUnanticipatedHostError );
     ENSURE_( SetApproximateSampleRate( pcm, hwParams, defaultSr ), paUnanticipatedHostError );
     ENSURE_( alsa_snd_pcm_hw_params_set_buffer_size_near( pcm, hwParams, &alsaBufferFrames ), paUnanticipatedHostError );
-    ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, &dir ), paUnanticipatedHostError );
+    ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, NULL ), paUnanticipatedHostError );
     *defaultHighLatency = (double) (alsaBufferFrames - alsaPeriodFrames) / defaultSr;

     *minChannels = (int)minChans;

[PortAudio-admin]

Comment by @RossBencina

proposed fix: https://app.assembla.com/spaces/portaudio/git/merge_requests/7612921?section=commits

[PortAudio-admin]

Comment by @RossBencina

There are three other places in the code where the dir variable is not initialized prior to calling an alsa function. The following merge request addresses the issue:

https://app.assembla.com/spaces/portaudio/git/merge_requests/7613051

[PortAudio-admin]

Comment by @philburk

The fix was merged by rbencina2020-08-05 17:25. Closing.

[PortAudio-admin]

Issue closed by @philburk