[vlc-devel] [vlc-commits] strings: use vlc_memstream in vlc_xml_encode()

[Rémi Denis-Courmont]

Le lauantaina 25. helmikuuta 2017, 9.22.30 EET Filip Roséen a écrit :
> Hi Rémi,
> 
> On 2017-02-25 10:11, Rémi Denis-Courmont wrote:
> > > Missing check to see if `vlc_memstream_open` actually created a valid
> > > handle.
> > 
> > No. It is, by design, safe to ignore vlc_memstream_open() failure.
> > Otherwise, I´d have marked it VLC_USED.
> 
> I know that it is safe, but I do not see any reason to ignore the
> *error-code*.

This error:
- cannot happen if the VLC implementation is used,
- is nigh-impossible to trigger with the native implementation.

Therefore, this will never be tested and never be run. There is no point in 
optimizing for a case that never happens (insofar as it is otherwise 
inconsequential). It only adds unnecessary complexity, platform dependent 
behaviour, red lines in coverage testing, and eventually over time, bugs in 
untested error paths.

> For cases where having a premature error-check means a more
> complicated implementation I do see a use for a "always ok"
> `vlc_memstream_open`, but for trivial cases I'd rather see things be
> checked - even though it is not mandatory.

No. By that same argument, we would also check errors at each and every 
vlc_memstream_*() write operation - and at least double the line count. I 
wrote the implementation on purpose so that checking for errors was entirely 
optional. The whole point of vlc_memstream is to _simplify_ formatting in 
memory.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/