Re: [PATCH v2] qmi: call-forwarding: fallback to basic forwarding info if no extended

Ivaylo Dimitrov <[email protected]> Wed, 11 Dec 2024 17:51:27 +0200
Newsgroups dev.linux.lists.ofono
Message-ID <[email protected]>
Hi Denis,

On 11.12.24 г. 7:27 ч., Denis Kenzior wrote:

...

> 
> const void *p;
> 

I'd better not, see below.

>>       uint8_t num;
>> +    const uint8_t *end;
>>       uint16_t length;
>> -
>> +    struct ofono_call_forwarding_condition *list = NULL;
> 
> A nicer way might be:
> _auto_(l_free) struct ofono_call_forwarding_condition *list = NULL;
> 
>> +    int i;
>> +    bool extended = false;
>>       DBG("");
>>       if (qmi_result_set_error(result, NULL))
>>           goto error;
>>       /*
>> -     * we want extended info, because of the number type.
>> -     * FIXME - shall we fallback to 0x10 if there is no extended info?
>> +     * we want extended info if any, because of the number type.
>>        */
>>       p = qmi_result_get(result, 0x16, &length);
>> -    if (p && length) {
>> -        struct ofono_call_forwarding_condition *list;
>> -        const uint8_t *end = p + length;
>> -        int i;
>> +    if (p && length)
>> +        extended = true;
>> +    else
>> +        p = qmi_result_get(result, 0x10, &length);
>> -        num = *p++;
>> +    if (!extended && (!p || !length))
>> +        goto error;
>> -        list = l_new(struct ofono_call_forwarding_condition, num);
>> +    end = p + length;

void * arithmetic, UB last time I checked, unless you are absolutely 
sure compilers will do the right thing we'd better keep it like that. p 
is used all over the place as const uint8_t *, I am not sure there is 
elegant way of converting it to void * without casts.

Will send v2 with fix for the above clang error and using _auto_.

Thanks,
Ivo