Re: [Qemu-devel] [PATCH] util/qemu-option: Document the get_opt_value() function

[Philippe Mathieu-Daudé]

On 9/19/19 12:58 PM, Damien Hedde wrote:
> Hi Philippe,
> 
> On 9/19/19 12:49 PM, Philippe Mathieu-Daudé wrote:
>> Coverity noticed commit 950c4e6c94 introduced a dereference before
>> null check in get_opt_value (CID1391003):
>>
>>   In get_opt_value: All paths that lead to this null pointer
>>   comparison already dereference the pointer earlier (CWE-476)
>>
>> We fixed this in commit 6e3ad3f0e31, but relaxed the check in commit
>> 0c2f6e7ee99 because "No callers of get_opt_value() pass in a NULL
>> for the 'value' parameter".
>>
>> Since this function is publicly exposed, it risks new users to do
>> the same error again. Avoid that documenting the 'value' argument
>> must not be NULL.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>  include/qemu/option.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/qemu/option.h b/include/qemu/option.h
>> index 844587cab3..141d6a883d 100644
>> --- a/include/qemu/option.h
>> +++ b/include/qemu/option.h
>> @@ -28,6 +28,18 @@
>>  
>>  #include "qemu/queue.h"
>>  
>> +/**
>> + * get_opt_value
>> + * @p: a pointer to the option name, delimited by commas
>> + * @value: a non-NULL pointer that will received the delimited options
>> + *
>> + * The @value char pointer will be allocated and filled with
>> + * the delimited options.
>> + * It is an error to pass a non-NULL @value parameter.
> 
> You mean "a NULL @value" I suppose (not a non-NULL).

Oops... Thanks :)

>> + *
>> + * Returns the position of the comma delimiter/zero byte after the
>> + * option name in @p.
>> + */
>>  const char *get_opt_value(const char *p, char **value);
>>  
>>  void parse_option_size(const char *name, const char *value,
>>
> 
> --
> Damien
>