-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, March 14, 2016 5:44 PM To: Lu, Han han.lu@intel.com Cc: liam.r.girdwood@linux.intel.com; Gautier, Bernard bernard.gautier@intel.com; Popescu, Edward C edward.c.popescu@intel.com; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 09/10] alsabat: use variable for thread return value
On Mon, 14 Mar 2016 10:36:53 +0100, Lu, Han wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, March 14, 2016 5:21 PM To: Lu, Han han.lu@intel.com Cc: liam.r.girdwood@linux.intel.com; Gautier, Bernard bernard.gautier@intel.com; Popescu, Edward C edward.c.popescu@intel.com; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [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@suse.de] Sent: Friday, March 11, 2016 9:34 PM To: Lu, Han han.lu@intel.com Cc: liam.r.girdwood@linux.intel.com; Gautier, Bernard bernard.gautier@intel.com; Popescu, Edward C edward.c.popescu@intel.com; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 09/10] alsabat: use variable for thread return value
On Wed, 02 Mar 2016 09:53:19 +0100, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Replace fixed "1" to variable for thread return value.
Signed-off-by: Lu, Han han.lu@intel.com
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;@@ -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;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
[han] OK. I'd like to send one patchset for 4 features: default name, standalone, tinyalsa and shell script, and send another patchset for trivial fixes.
BR, Han