Hi Remi,<br><br>On Tuesday, November 1, 2016, Rémi Denis-Courmont <<a href="mailto:remi@remlab.net">remi@remlab.net</a>> wrote:<br>> Le tiistaina 1. marraskuuta 2016, 2.33.03 EET Filip Roséen a écrit :<br>>> FromACP is used to convert a c-style string from one charset to the<br>>> other, as such it does not make sense to call the function with NULL<br>>> (especially given that NULL is not a valid input for the function).<br>>><br>>> These changes fixes the checks to see whether or not the function is<br>>> applicable or not by properly checking the argument that would-be<br>>> passed (instead of just the object that contains it).<br>>><br>>> fixes #17581<br>><br>> IMO, in this case, it seems simpler to accept NULL in local function<br>> FromACP().<br><br>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.<br><br>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").<br><br>This turned into a little bit of a rant I guess, but I am on a bus with nothing to do.<br><br>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.<br><br>> --<br>> Rémi Denis-Courmont<br>> <a href="http://www.remlab.net/">http://www.remlab.net/</a><br>><br>> _______________________________________________<br>> vlc-devel mailing list<br>> To unsubscribe or modify your subscription options:<br>> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br>>