[vlc-devel] [PATCH 2/2] include: strings: Add size check to vlc_hex_encode_binary

[Steve Lhomme]

On 2020-04-16 20:58, Thomas Guillem wrote:
> 
> 
> On Thu, Apr 16, 2020, at 16:05, Marvin Scholz wrote:
>> ---
>>   include/vlc_hash.h    | 16 ++++++++--------
>>   include/vlc_strings.h |  5 +++--
>>   src/text/strings.c    |  6 ++++--
>>   3 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/vlc_hash.h b/include/vlc_hash.h
>> index 1e7757ac58..b5136677d2 100644
>> --- a/include/vlc_hash.h
>> +++ b/include/vlc_hash.h
>> @@ -57,14 +57,14 @@
>>    * \param[out]    output Output buffer to write the string to
>>    */
>>   #ifndef __cplusplus
>> -#define vlc_hash_FinishHex(ctx, output, size)                       \
>> -    do {                                                            \
>> -        char out_tmp[_Generic((ctx),                                \
>> -            vlc_hash_md5_t *: VLC_HASH_MD5_DIGEST_SIZE)];           \
>> -        _Generic((ctx),                                             \
>> -            vlc_hash_md5_t *: vlc_hash_md5_Finish)                  \
>> -        (ctx, out_tmp, sizeof(out_tmp));                            \
>> -        vlc_hex_encode_binary(out_tmp, sizeof(out_tmp), output);    \
>> +#define vlc_hash_FinishHex(ctx, output, size)                           \
>> +    do {                                                                \
>> +        char out_tmp[_Generic((ctx),                                    \
>> +            vlc_hash_md5_t *: VLC_HASH_MD5_DIGEST_SIZE)];               \
>> +        _Generic((ctx),                                                 \
>> +            vlc_hash_md5_t *: vlc_hash_md5_Finish)                      \
>> +        (ctx, out_tmp, sizeof(out_tmp));                                \
>> +        vlc_hex_encode_binary(out_tmp, sizeof(out_tmp), output, size);  \
>>       } while (0)
>>   #endif
>>   
>> diff --git a/include/vlc_strings.h b/include/vlc_strings.h
>> index 930d3347fb..5b4436141f 100644
>> --- a/include/vlc_strings.h
>> +++ b/include/vlc_strings.h
>> @@ -125,10 +125,11 @@ VLC_API char *vlc_xml_encode(const char *str)
>> VLC_MALLOC;
>>    * string in hexadecimal representation.
>>    *
>>    * \param      input    Input buffer
>> - * \param      size     Input buffer size
>> + * \param      in_size  Input buffer size
>>    * \param[out] output   Output buffer to write the string to
>> + * \param      out_size Output buffer size
>>    */
>> -VLC_API void vlc_hex_encode_binary(const void *input, size_t size,
>> char *output);
>> +VLC_API void vlc_hex_encode_binary(const void *input, size_t in_size,
>> char *output, size_t out_size);
>>   
>>   /**
>>    * Base64 encoding.
>> diff --git a/src/text/strings.c b/src/text/strings.c
>> index 9676687aae..fc0babc7ce 100644
>> --- a/src/text/strings.c
>> +++ b/src/text/strings.c
>> @@ -347,11 +347,13 @@ char *vlc_xml_encode (const char *str)
>>   }
>>   
>>   /* Hex encoding */
>> -void vlc_hex_encode_binary(const void *input, size_t size, char *output)
>> +void vlc_hex_encode_binary(const void *input, size_t in_size,
>> +                           char *output, size_t out_size)
>>   {
>> +    assert(out_size >= in_size * 2 + 1); // 2 chars per byte + null
> 
> Missing a (void) out_size; to clear warning on builds with NDEBUG.

Can we just disable this warning ? We keep adding dummy code just to 
silence this warning. I don't think there are cases where it's useful to 
know we don't use a variable (apart when cleaning an API but then a grep 
is still needed to verify all the calls anyway).

> Otherwise, LGTM for the set.
> 
>>       const unsigned char *buffer = input;
>>   
>> -    for (size_t i = 0; i < size; i++) {
>> +    for (size_t i = 0; i < in_size; i++) {
>>           sprintf(&output[i * 2], "%02hhx", buffer[i]);
>>       }
>>   }
>> -- 
>> 2.24.1 (Apple Git-126)
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>