[vlc-devel] [vlc-commits] strings: use vlc_memstream in vlc_xml_encode()
filip at atch.se
Sat Feb 25 15:05:21 CET 2017
On 2017-02-25 15:27, Rémi Denis-Courmont wrote:
> > > The explanation is factual. One does not get to agree or disagree with it.
> > Ok, now I completely am out of words. I thought this was a discussion,
> > not a place where "factual" statements expressed by one party are the
> > only truth, and opinions of the other party does not matter.
> - It is a fact, not an opinion, that ignoring an error here is safe.
> - It is a fact, not an opinion, that an error cannot happen with one back-end.
> - It is a fact, not an opinion, that an error is practically impossible to
> trigger with the other back-end.
Just as practically impossible as memory-allocation failures.
> It is therefore also a fact that adding the error handling there will:
> - make the source code marginally more complex,
> - make the binary code marginally slower,
> - make code coverage worse.
> And it is an extremely likely conjecture if not a proven fact that the error
> - will not be tested much if at all,
> - will become a (good or bad) example for future code.
> Also the commit message is misleading since the error check while absent is
> not "missing". And the code might be revectored or copied in circumstances
> that would require less trivial and more bug-prone handling. That too would
> normally be a conjecture rather than a fact, except that this has already
> occurred and been proven true in other code paths (TTML, MRL helpers).
You did not even reply to the section regarding that, and I still do
not see how the implementation is so non-trivial that it is a point
you bring up against error-checking.
> > > > > > `vlc_memstream_open` can fail. If you consider it so unlikely
> > > > > > that one should not bother checking the *return-value* I do not see
> > > > > > why you put it there in the first place.
> > > > >
> > > > > This is a red herring.
> > > >
> > > > It is very much relevant to the discussion and cuts straight to what
> > > > you wrote about the unlikelihood of the function to fail (and not
> > > > having to check it).
> > >
> > > You are again making the false dichotomy which you yourself deny.
> > > There is an error return because there is a possible error case.
> > > There is no "missing check" (and no VLC_USED qualifier) because the
> > > error can be safely ignored.
> > I think I have explained my ground in previous posts, but to
> > summarize:
> > - I do not see why it is so bad introducing checks for the
> > *return-value* (because I do not agree with your statement
> > regarding how non-trivial an implemenation becomes), and;
> I never said it was "so bad". Indeed, it does not add any immediate bug. I do
> believe that it makes the code worse rather than better, and thus am rejecting
> the patch.
I will leave these two links here for future readers:
> > - I certainly do not see why removing the *return-value* would be
> > foolish (when actually handling it is, as stated, wrong in most
> > (all) cases).
> If an error is possible, it looks perfectly normal and reasonable to return an
> error status, even if the error can safely be ignored by the caller. I do not
> see why that specific function would be any different from the myriad of
> functions that follow the same pattern (in VLC and elsewhere).
> You seem to imply that error handling should either be mandatory or forbidden.
That's not my opinion, my opinion (as stated numerous times) is that
the error-check is not invalid nor complicated enough to warrant it to
> But I did purposedly leave error handling *optional* in this case, because it
> added no practical cost on the implementation(*) on the one hand, and gave
> flexibility to the user on the other hand.
> And FWIW, it is Thomas, not I, who decided to make use the second option by
> handling the error. All three original use cases ignored it.
> And I am very very very fed up with this discussion so that will be my last
You are certainly not the only one, and I am still interested in the
remarks you made regarding my contributions and the quality of what I
do (because I consider what you wrote very harsh, and not at all
inspiring in terms of future contribution).
As most often, I am available on IRC if you feel like clearifying.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel