| Message ID | 20180316144047.30904-1-nia.alarie@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/16/2018 09:40 AM, Nia Alarie wrote: > If qemu_strtoi indicates an error, return the default value. Would it be better to diagnose the error instead of silently returning a default value? > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com> > --- > audio/audio.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index 6eccdb17ee..d6e91901aa 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int defval, int *defaultp) > char *strval; > > strval = getenv (key); > - if (strval) { > + if (strval && !qemu_strtoi(strval, NULL, 10, &val)) { > *defaultp = 0; > - val = atoi (strval); > return val; > } > else { >
On Mon, Mar 19, 2018 at 2:47 PM, Eric Blake <eblake@redhat.com> wrote: > On 03/16/2018 09:40 AM, Nia Alarie wrote: >> >> If qemu_strtoi indicates an error, return the default value. > > > Would it be better to diagnose the error instead of silently returning a > default value? > >> >> Signed-off-by: Nia Alarie <nia.alarie@gmail.com> >> --- >> audio/audio.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index 6eccdb17ee..d6e91901aa 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int >> defval, int *defaultp) >> char *strval; >> strval = getenv (key); >> - if (strval) { >> + if (strval && !qemu_strtoi(strval, NULL, 10, &val)) { >> *defaultp = 0; >> - val = atoi (strval); >> return val; >> } >> else { >> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org Possibly, while writing these patches I was just going by what was already there. I can see how that would be good. Should the code provide a warning to the user and continue with the default, or provide the warning and exit? And is it more correct to use dolog() or AUD_log() in this context?
On 03/19/2018 10:01 AM, nee wrote: > On Mon, Mar 19, 2018 at 2:47 PM, Eric Blake <eblake@redhat.com> wrote: >> On 03/16/2018 09:40 AM, Nia Alarie wrote: >>> >>> If qemu_strtoi indicates an error, return the default value. >> >> >> Would it be better to diagnose the error instead of silently returning a >> default value? >> > > Possibly, while writing these patches I was just going by what was > already there. I can see how that would be good. > > Should the code provide a warning to the user and continue with the > default, or provide the warning and exit? And is it more correct to > use dolog() or AUD_log() in this context? I'll defer to the audio maintainer's opinion on what might be best here. But my personal preference is that if the only time you can give invalid input is via a bad command line argument, then print the error and exit immediately (the VM never starts), so that the user can then fix their bad command line and get the value they wanted instead of silently running with a different value all because of a typo that caused us to fail to parse a number.
diff --git a/audio/audio.c b/audio/audio.c index 6eccdb17ee..d6e91901aa 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int defval, int *defaultp) char *strval; strval = getenv (key); - if (strval) { + if (strval && !qemu_strtoi(strval, NULL, 10, &val)) { *defaultp = 0; - val = atoi (strval); return val; } else {
If qemu_strtoi indicates an error, return the default value. Signed-off-by: Nia Alarie <nia.alarie@gmail.com> --- audio/audio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)