<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Rémi,</p>
<p>On 2017-02-25 10:36, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Le lauantaina 25. helmikuuta 2017, 9.22.30 EET Filip Roséen a écrit :</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Hi Rémi,
On 2017-02-25 10:11, Rémi Denis-Courmont wrote:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Missing check to see if `vlc_memstream_open` actually created a valid
handle.</code></pre>
</blockquote>
<pre><code> No. It is, by design, safe to ignore vlc_memstream_open() failure.
Otherwise, I´d have marked it VLC_USED.</code></pre>
</blockquote>
<pre><code> I know that it is safe, but I do not see any reason to ignore the
*error-code*.</code></pre>
</blockquote>
<pre><code> This error:
- cannot happen if the VLC implementation is used,
- is nigh-impossible to trigger with the native implementation.</code></pre>
</blockquote>
<p>But the VLC implemementation is not the only one, so that argument is moot. <code>vlc_memstream_open</code> can fail. If you consider it so unlikely that one should not bother checking the <em>return-value</em> I do not see why you put it there in the first place.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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.</code></pre>
</blockquote>
<p>Then remove the return-value all together; it is contradicting to have it with your expressed opinions.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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.</code></pre>
</blockquote>
<pre><code> 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.</code></pre>
</blockquote>
<p>It is trivial to check whether the initialization of the handle was successful, it is however not trivial to have an if for every write-operation (I have never claimed that I would recommend that, nor have I done so when I have used the relevant functions).</p>
<p>It surprises me to see your stance on this matter given that you have yourself checked the <em>error-code</em> in <code>e9804d78697</code>; what was the rationale in that case (if any)?</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> The whole point of vlc_memstream is to _simplify_ formatting in memory.</code></pre>
</blockquote>
<p>To check whether <code>vlc_memstream_open</code> is successful or not certainly does not make the usage of <code>struct vlc_memstream</code> as complicated as you seem to imply it does.</p>
</body>
</html>