<p>On 16/05/11 17:11, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<p>We do usually care if variables are clobbered. There are not so old examples of bugs in the VLC code base due to this. In this case, the variable cannot be clobered, for a very simple reason: the long jump is never actually performed.</p>
</blockquote>
<p>I might have wrongfully assumed that there actually could be a path where the longjmp would take place, but now that I look into the issue more carefully there is indeed no cancellation point outside the scope (invoked by the function in question).</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<p>This trick might work in this specific code path with your specific compiler and libc versions and settings. But it won’t work in other cases because in general long jump cloberred variables may need to be declared before the setjmp() call.</p>
</blockquote>
<p>But since this variable is in no way part of the actual cleanup, why do we need it to be declared prior to doing vlc_fifo_CleanupPush (which indirectly does what has been previously described)?</p>
<p>If the variable is in a more narrow scope we cannot observe if the variable is clobbered or not, and since we in this case does not (and should not) care about the value I do not see the immediate problem with the patch.</p>
<p>Out of pure curiousity: could you elaborate why this patch is wrong?</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<p>I fail to see a bug in VLC code here. This is an issue between GCC and glibc, and so we should not put ugly hacks in the VLC code base for it. If those warnings annoy you, turn them off.</p>
</blockquote>
<p>I am currently working on the code responsible for the preroll, the warning pops up every time I recompile src/input/decoder.c, and I got tired of having it nag me each time.</p>
<p>If we could get rid of the warning I’d be a happy camper, but if applying this patch might become a danger to the codebase as a whole.. well, it’s obvious that we should leave the implementation as-is.</p>
<p>Turning it off locally is not something I feel very comfortable with, since I’d like to view the codebase as it is (upstream); so if this is indeed the wrong path to walk, I’d rather have the diagnostic remind me on each compilation.</p>
<pre><code>;-)</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<p>Ideally, we would turn them off globally, but we would first have to get rid of all <em>direct</em> sigsetjmp/setjmp usage in VLC.</p>
</blockquote>
<p>Makes sense, thanks for the quick reply.</p>