[vlc-devel] [PATCH] demux/avi: fix 17581: do not call FromACP with NULL

Filip Roséen filip at atch.se
Tue Nov 1 15:35:32 CET 2016

Hi Remi,

On Tuesday, November 1, 2016, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le tiistaina 1. marraskuuta 2016, 2.33.03 EET Filip Roséen a écrit :
>> FromACP is used to convert a c-style string from one charset to the
>> other, as such it does not make sense to call the function with NULL
>> (especially given that NULL is not a valid input for the function).
>> These changes fixes the checks to see whether or not the function is
>> applicable or not by properly checking the argument that would-be
>> passed (instead of just the object that contains it).
>> fixes #17581
> IMO, in this case, it seems simpler to accept NULL in local function
> FromACP().

In terms of limiting the number of changes, yes; it would be simpler.
Though, another personal opinion, I am not a big fan of a function
protecting itself from inaccurate usage; it should be the callee who makes
sure that the function is called with applicable arguments.

In my opinion, it does not make sense calling a function that does
conversion from some data-format to some other data-format without any data
(it's like invoking memcpy with NULL, expecting memcpy to protect against
"no data").

This turned into a little bit of a rant I guess, but I am on a bus with
nothing to do.

IMO, if there's anything missing from the implementation it is an assert in
FromACP - though the changes in the patch was not primarily aimed at
code-cleanup (in that regard) - but bug fixing.

> --
> Rémi Denis-Courmont
> http://www.remlab.net/
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161101/180b879c/attachment.html>

More information about the vlc-devel mailing list