[alsa-devel] [PATCH 0/3] *** Some trival fixes on alsabat ***
From: "Lu, Han" han.lu@intel.com
Fix misusing of errno, fopen, prints and return value
Lu, Han (3): alsabat: fix misusing of errno alsabat: fix fopen and messages alsabat: use variable for thread return value
bat/alsa.c | 50 ++++++++++++++++++++++++++------------------------ bat/analyze.c | 6 +++--- bat/bat.c | 39 ++++++++++++++++++++++----------------- bat/common.c | 1 - 4 files changed, 51 insertions(+), 45 deletions(-)
From: "Lu, Han" han.lu@intel.com
Preserve errno value before use, since the value might be changed by another library call. Add "#include <errno.h>" and remove redundant include.
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/bat/alsa.c b/bat/alsa.c index 5775748..638012a 100644 --- a/bat/alsa.c +++ b/bat/alsa.c @@ -19,6 +19,7 @@ #include <math.h> #include <stdint.h> #include <pthread.h> +#include <errno.h>
#include <alsa/asoundlib.h>
@@ -300,17 +301,17 @@ static int write_to_pcm_loop(struct pcm_container *sndpcm, struct bat *bat)
if (bat->debugplay) { fp = fopen(bat->debugplay, "wb"); + err = -errno; if (fp == NULL) { fprintf(bat->err, _("Cannot open file for capture: ")); - fprintf(bat->err, _("%s %d\n"), bat->debugplay, -errno); - return -errno; + fprintf(bat->err, _("%s %d\n"), bat->debugplay, err); + return err; } /* leave space for wav header */ - err = fseek(fp, sizeof(wav), SEEK_SET); - if (err != 0) { - fprintf(bat->err, _("Seek file error: %d %d\n"), - err, -errno); - return -errno; + if (fseek(fp, sizeof(wav), SEEK_SET) != 0) { + err = -errno; + fclose(fp); + return err; } }
@@ -401,10 +402,11 @@ void *playback_alsa(struct bat *bat) fprintf(bat->log, _("Playing input audio file: %s\n"), bat->playback.file); bat->fp = fopen(bat->playback.file, "rb"); + err = -errno; if (bat->fp == NULL) { fprintf(bat->err, _("Cannot open file for capture: ")); fprintf(bat->err, _("%s %d\n"), - bat->playback.file, -errno); + bat->playback.file, err); retval_play = 1; goto exit3; } @@ -544,9 +546,10 @@ void *record_alsa(struct bat *bat)
remove(bat->capture.file); fp = fopen(bat->capture.file, "w+"); + err = -errno; if (fp == NULL) { fprintf(bat->err, _("Cannot open file for capture: %s %d\n"), - bat->capture.file, -errno); + bat->capture.file, err); retval_record = 1; goto exit3; } diff --git a/bat/analyze.c b/bat/analyze.c index 5cfdac3..63481fb 100644 --- a/bat/analyze.c +++ b/bat/analyze.c @@ -294,10 +294,10 @@ int analyze_capture(struct bat *bat) return -ENOMEM;
bat->fp = fopen(bat->capture.file, "rb"); + err = -errno; if (bat->fp == NULL) { fprintf(bat->err, _("Cannot open file for capture: %s %d\n"), - bat->capture.file, -errno); - err = -errno; + bat->capture.file, err); goto exit1; }
diff --git a/bat/bat.c b/bat/bat.c index 85ec5aa..4821532 100644 --- a/bat/bat.c +++ b/bat/bat.c @@ -39,13 +39,15 @@
static int get_duration(struct bat *bat) { + int err; float duration_f; long duration_i; char *ptrf, *ptri;
duration_f = strtof(bat->narg, &ptrf); + err = -errno; if (duration_f == HUGE_VALF || duration_f == -HUGE_VALF - || (duration_f == 0.0 && errno != 0)) + || (duration_f == 0.0 && err != 0)) goto err_exit;
duration_i = strtol(bat->narg, &ptri, 10); @@ -68,9 +70,9 @@ static int get_duration(struct bat *bat) return 0;
err_exit: - fprintf(bat->err, _("Duration overflow/underflow: %d\n"), -errno); + fprintf(bat->err, _("Duration overflow/underflow: %d\n"), err);
- return -errno; + return err; }
static void get_sine_frequencies(struct bat *bat, char *freq) @@ -458,17 +460,18 @@ static int validate_options(struct bat *bat) static int bat_init(struct bat *bat) { int err = 0; + int fd = 0; char name[] = TEMP_RECORD_FILE_NAME;
/* Determine logging to a file or stdout and stderr */ if (bat->logarg) { bat->log = NULL; bat->log = fopen(bat->logarg, "wb"); + err = -errno; if (bat->log == NULL) { fprintf(bat->err, _("Cannot open file for capture:")); - fprintf(bat->err, _(" %s %d\n"), - bat->logarg, -errno); - return -errno; + fprintf(bat->err, _(" %s %d\n"), bat->logarg, err); + return err; } bat->err = bat->log; } @@ -489,18 +492,20 @@ static int bat_init(struct bat *bat) bat->capture.file = bat->playback.file; } else { /* create temp file for sound record and analysis */ - err = mkstemp(name); - if (err == -1) { + fd = mkstemp(name); + err = -errno; + if (fd == -1) { fprintf(bat->err, _("Fail to create record file: %d\n"), - -errno); - return -errno; + err); + return err; } /* store file name which is dynamically created */ bat->capture.file = strdup(name); + err = -errno; if (bat->capture.file == NULL) - return -errno; + return err; /* close temp file */ - close(err); + close(fd); }
/* Initial for playback */ @@ -526,11 +531,12 @@ static int bat_init(struct bat *bat) } } else { bat->fp = fopen(bat->playback.file, "rb"); + err = -errno; if (bat->fp == NULL) { fprintf(bat->err, _("Cannot open file for playback:")); fprintf(bat->err, _(" %s %d\n"), - bat->playback.file, -errno); - return -errno; + bat->playback.file, err); + return err; } err = read_wav_header(bat, bat->playback.file, bat->fp, false); fclose(bat->fp); diff --git a/bat/common.c b/bat/common.c index 798b00b..41aaf3a 100644 --- a/bat/common.c +++ b/bat/common.c @@ -17,7 +17,6 @@ #include <stddef.h> #include <stdlib.h> #include <stdbool.h> -#include <errno.h>
#include "aconfig.h" #include "gettext.h"
From: "Lu, Han" han.lu@intel.com
All files should be opened in either "rb" or "wb" in current usage. Remove incorrect and unneccesary prints.
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/bat/alsa.c b/bat/alsa.c index 638012a..2540ab8 100644 --- a/bat/alsa.c +++ b/bat/alsa.c @@ -303,8 +303,8 @@ static int write_to_pcm_loop(struct pcm_container *sndpcm, struct bat *bat) fp = fopen(bat->debugplay, "wb"); err = -errno; if (fp == NULL) { - fprintf(bat->err, _("Cannot open file for capture: ")); - fprintf(bat->err, _("%s %d\n"), bat->debugplay, err); + fprintf(bat->err, _("Cannot open file: %s %d\n"), + bat->debugplay, err); return err; } /* leave space for wav header */ @@ -404,8 +404,7 @@ void *playback_alsa(struct bat *bat) bat->fp = fopen(bat->playback.file, "rb"); err = -errno; if (bat->fp == NULL) { - fprintf(bat->err, _("Cannot open file for capture: ")); - fprintf(bat->err, _("%s %d\n"), + fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->playback.file, err); retval_play = 1; goto exit3; @@ -545,10 +544,10 @@ void *record_alsa(struct bat *bat) }
remove(bat->capture.file); - fp = fopen(bat->capture.file, "w+"); + fp = fopen(bat->capture.file, "wb"); err = -errno; if (fp == NULL) { - fprintf(bat->err, _("Cannot open file for capture: %s %d\n"), + fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->capture.file, err); retval_record = 1; goto exit3; diff --git a/bat/analyze.c b/bat/analyze.c index 63481fb..58781d6 100644 --- a/bat/analyze.c +++ b/bat/analyze.c @@ -296,7 +296,7 @@ int analyze_capture(struct bat *bat) bat->fp = fopen(bat->capture.file, "rb"); err = -errno; if (bat->fp == NULL) { - fprintf(bat->err, _("Cannot open file for capture: %s %d\n"), + fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->capture.file, err); goto exit1; } diff --git a/bat/bat.c b/bat/bat.c index 4821532..85a7282 100644 --- a/bat/bat.c +++ b/bat/bat.c @@ -469,8 +469,8 @@ static int bat_init(struct bat *bat) bat->log = fopen(bat->logarg, "wb"); err = -errno; if (bat->log == NULL) { - fprintf(bat->err, _("Cannot open file for capture:")); - fprintf(bat->err, _(" %s %d\n"), bat->logarg, err); + fprintf(bat->err, _("Cannot open file: %s %d\n"), + bat->logarg, err); return err; } bat->err = bat->log; @@ -533,8 +533,7 @@ static int bat_init(struct bat *bat) bat->fp = fopen(bat->playback.file, "rb"); err = -errno; if (bat->fp == NULL) { - fprintf(bat->err, _("Cannot open file for playback:")); - fprintf(bat->err, _(" %s %d\n"), + fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->playback.file, err); return err; }
From: "Lu, Han" han.lu@intel.com
Use variable instead of 0/1 to indicate the return value of playback and capture threads.
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/bat/alsa.c b/bat/alsa.c index 2540ab8..79c86fe 100644 --- a/bat/alsa.c +++ b/bat/alsa.c @@ -383,13 +383,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; }
@@ -406,20 +406,20 @@ void *playback_alsa(struct bat *bat) if (bat->fp == NULL) { fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->playback.file, err); - retval_play = 1; + retval_play = err; goto exit3; } /* Skip header */ err = read_wav_header(bat, bat->playback.file, bat->fp, true); if (err != 0) { - retval_play = 1; + retval_play = err; goto exit4; } }
err = write_to_pcm_loop(&sndpcm, bat); if (err != 0) { - retval_play = 1; + retval_play = err; goto exit4; }
@@ -533,13 +533,13 @@ void *record_alsa(struct bat *bat) if (err != 0) { fprintf(bat->err, _("Cannot open PCM capture device: ")); fprintf(bat->err, _("%s(%d)\n"), snd_strerror(err), err); - retval_record = 1; + retval_record = err; goto exit1; }
err = set_snd_pcm_params(bat, &sndpcm); if (err != 0) { - retval_record = 1; + retval_record = err; goto exit2; }
@@ -549,7 +549,7 @@ void *record_alsa(struct bat *bat) if (fp == NULL) { fprintf(bat->err, _("Cannot open file: %s %d\n"), bat->capture.file, err); - retval_record = 1; + retval_record = err; goto exit3; }
@@ -563,7 +563,7 @@ void *record_alsa(struct bat *bat)
err = write_wav_header(fp, &wav, bat); if (err != 0) { - retval_record = 1; + retval_record = err; goto exit4; }
@@ -571,7 +571,7 @@ void *record_alsa(struct bat *bat) fprintf(bat->log, _("Recording ...\n")); err = read_from_pcm_loop(fp, count, &sndpcm, bat); if (err != 0) { - retval_record = 1; + retval_record = err; goto exit4; }
On Mon, 21 Mar 2016 12:05:46 +0100, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Fix misusing of errno, fopen, prints and return value
Lu, Han (3): alsabat: fix misusing of errno alsabat: fix fopen and messages alsabat: use variable for thread return value
Applied all three patches now. Thanks.
Takashi
bat/alsa.c | 50 ++++++++++++++++++++++++++------------------------ bat/analyze.c | 6 +++--- bat/bat.c | 39 ++++++++++++++++++++++----------------- bat/common.c | 1 - 4 files changed, 51 insertions(+), 45 deletions(-)
-- 2.5.0
participants (2)
-
han.lu@intel.com
-
Takashi Iwai