Re: [PATCH 5/8] ipconfig: Fix Clang garbage value complaints

Jussi Laakkonen <[email protected]> Tue, 15 Apr 2025 13:47:54 +0300
Newsgroups dev.linux.lists.connman
Message-ID <[email protected]>
Hi Denis,

On 4/14/25 18:42, Denis Kenzior wrote:
> Hi Jussi,
> 
> On 4/10/25 10:58 AM, Jussi Laakkonen wrote:
>> ---
>>   src/ipconfig.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/ipconfig.c b/src/ipconfig.c
>> index c2d7d2bc..ce4bbc27 100644
>> --- a/src/ipconfig.c
>> +++ b/src/ipconfig.c
>> @@ -3,6 +3,7 @@
>>    *  Connection Manager
>>    *
>>    *  Copyright (C) 2007-2013  Intel Corporation. All rights reserved.
>> + *  Copyright (C) 2025  Jolla Mobile Ltd
>>    *
>>    *  This program is free software; you can redistribute it and/or 
>> modify
>>    *  it under the terms of the GNU General Public License version 2 as
>> @@ -325,8 +326,10 @@ static int read_conf_value(const char *prefix, 
>> const char *ifname,
>>       // null-terminated variable argument component list.
>>
>>       path = g_build_filename(prefix, ifname ? ifname : "all", suffix, 
>> NULL);
>> -    if (!path)
>> +    if (!path) {
>> +        *value = -ENOMEM;
>>           return -ENOMEM;
>> +    }
> 
> Looking at g_build_filename, pretty sure it also never fails.  It is 
> customary to not-side effect any inout arguments if the function returns 
> an error, so setting *value here is probably incorrect?

Now that I rethink of this, yes, I guess another trap of 
paranoid-Clang-analyzer. This can be skipped.

> 
>>
>>       errno = 0; /* Avoid stale errno values with fopen */
>>       f = fopen(path, "r");
>> @@ -337,7 +340,7 @@ static int read_conf_value(const char *prefix, 
>> const char *ifname,
>>
>>           err = fscanf(f, "%d", value);
>>           if (err <= 0 && errno)
>> -            err = -errno;
>> +            *value = err = -errno;
> 
> As above.

Yes. I agree.


> 
>>
>>           fclose(f);
>>       }
>> -- 
>> 2.39.5
>>
>>
> 
- Jussi