On Mon, 14 Mar 2016 10:36:53 +0100, Lu, Han wrote: > > Hi Takashi, > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > Sent: Monday, March 14, 2016 5:21 PM > > To: Lu, Han <han.lu@xxxxxxxxx> > > Cc: liam.r.girdwood@xxxxxxxxxxxxxxx; Gautier, Bernard > > <bernard.gautier@xxxxxxxxx>; Popescu, Edward C > > <edward.c.popescu@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx > > Subject: Re: [PATCH 09/10] alsabat: use variable for thread > > return value > > > > On Mon, 14 Mar 2016 10:15:20 +0100, > > Lu, Han wrote: > > > > > > Hi Takashi, > > > > > > > -----Original Message----- > > > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > > Sent: Friday, March 11, 2016 9:34 PM > > > > To: Lu, Han <han.lu@xxxxxxxxx> > > > > Cc: liam.r.girdwood@xxxxxxxxxxxxxxx; Gautier, Bernard > > > > <bernard.gautier@xxxxxxxxx>; Popescu, Edward C > > > > <edward.c.popescu@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx > > > > Subject: Re: [PATCH 09/10] alsabat: use variable for > > > > thread return value > > > > > > > > On Wed, 02 Mar 2016 09:53:19 +0100, > > > > han.lu@xxxxxxxxx wrote: > > > > > > > > > > From: "Lu, Han" <han.lu@xxxxxxxxx> > > > > > > > > > > Replace fixed "1" to variable for thread return value. > > > > > > > > > > Signed-off-by: Lu, Han <han.lu@xxxxxxxxx> > > > > > > > > > > diff --git a/bat/alsa.c b/bat/alsa.c index 0a5f899..189b0e9 100644 > > > > > --- a/bat/alsa.c > > > > > +++ b/bat/alsa.c > > > > > @@ -309,13 +309,13 @@ void *playback_alsa(struct bat *bat) > > > > > if (err != 0) { > > > > > fprintf(bat->err, _("Cannot open PCM playback device: ")); > > > > > fprintf(bat->err, _("%s(%d)\n"), snd_strerror(err), err); > > > > > - retval_play = 1; > > > > > + retval_play = err; > > > > > goto exit1; > > > > > } > > > > > > > > > > err = set_snd_pcm_params(bat, &sndpcm); > > > > > if (err != 0) { > > > > > - retval_play = 1; > > > > > + retval_play = err; > > > > > goto exit2; > > > > > } > > > > > > > > > > @@ -332,13 +332,13 @@ void *playback_alsa(struct bat *bat) > > > > > fprintf(bat->err, _("Cannot open file for playback: ")); > > > > > fprintf(bat->err, _("%s %d\n"), > > > > > bat->playback.file, -errno); > > > > > - retval_play = 1; > > > > > + retval_play = -errno; > > > > > > > > Is the original errno still preserved at this point...? > > > > > > [han] I think so, the complete code section here is > > > ... > > > bat->fp = fopen(bat->playback.file, "rb"); > > > if (bat->fp == NULL) { > > > fprintf(bat->err, _("Cannot open file for playback: ")); > > > fprintf(bat->err, _("%s %d\n"), > > > bat->playback.file, -errno); > > > - retval_play = 1; > > > + retval_play = -errno; > > > goto exit3; > > > } > > > ... > > > So the use of errno should be safe. > > > > You call a bunch of functions between the fopen() error and the reference of > > errno. The errno reference should be done immediately after the error. > > > > > > Takashi > > [han] sorry! I ignored the errno could be changed by the printf(). I'll rewrite the > patch and fix other similar errors. While we're at it: at the next respin, could you split the patchset, one for trivial fixes that can be applied immediately, and another patchset for extending the feature? Mixing up both fixes and enhancements made it difficult to pick up each patch. In that way, I'll be able to apply the fix patches more quickly while reviewing the enhancement patches more intensively. thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
