[vlc-devel] [PATCH] Add open_memstream replacement

Derek Buitenhuis derek.buitenhuis at gmail.com
Thu Aug 25 14:19:45 CEST 2016


On 8/25/2016 4:57 AM, Rémi Denis-Courmont wrote:
> Le keskiviikkona 24. elokuuta 2016, 17.43.03 EEST Derek Buitenhuis a écrit :
>> [17:34] < Daemon404> use snprintf, check its return value, realloc if
>> needed, continue. [17:38]
> 
> That´s called (v)asprintf() and we already use it where it´s reasonable. We 
> also sometimes use it where it´s not so reasonable and results in monster 
> format strings, that are illegible, have conveyed bugs times and times, and 
> presuambly increase the complexity score.

[...]

> With the current vlc_uri_compose(), I counted 104 different combination of 
> formats, and it doesn´t even support URI fragments yet. With the other call 
> sites, I can´t even see how you´d use printf() directly as we can´t express 
> loops within a single printf() function call.

I did not suggest solely to use format printing. There's no reason vlc_uri_compose
can't be implemented using various (including non-printf!) functions, other than
"it's inconvenient".

> I only know three string buffers in C, open_memstream/fmemopen (POSIX), 
> funopen (BSD) and fopencookie (GNU). The first one is the most standard. And 
> perhaps more importantly it is also the least generic, so it can be 
> reimplemented with the other two. The reverse is not true.

And it seems pretty silly to me to include entire 200 line reimplemntations
and also not supporting Windows, in order to use the latter two, when it
is not 100% required. Of course, I am not: a) A VLC developer or b) A VideoLAN
Board member, so my opniion isn't worth much here; I'm the peanut gallery.

> Now if you know a better C string buffers API, please advise. snprintf() is 
> NOT a better C string buffers API, and I don´t suppose you suggest switching 
> to C++.

[...]

>> < Daemon404> (also, fyi, the current
>> open_memstream based impl will fail hard if there is an alloc failure,
>> since all the writes to the buffer are unchecked)
> 
> I fail to see how realloc() will make a difference there. If memory allocation 
> fails, the best you can do is hope the process won´t be killed and return an 
> error to the caller (which we do, AFAIK).

I meant this in a "the current implementation will already fail on
alloc failure" way, not to suggest that realloc is better at this.
I do think a realloc() failure is easier checked in a more explicitly
way than a memory back FILE having a realloc failure, though. As above,
I'm not a VLC developer, so I don't know VLC's policies on explicit
OOM/allco failure checking.

- Derek


More information about the vlc-devel mailing list