[Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi

[Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi

[Nia Alarie]

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(-)

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 {
-- 
2.16.2

Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi

[Eric Blake]

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

Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi

[nee]

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?

Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi

[Eric Blake]

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org