[vlc-devel] [PATCH] core: moved variable declaration to prevent compiler warning (gcc -Wclobbered)

Filip Roséen filip at videolabs.io
Wed May 11 17:23:32 CEST 2016


On 16/05/11 17:11, Rémi Denis-Courmont wrote:

> 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.

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).

> 
> 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.

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)?

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.

Out of pure curiousity: could you elaborate why this patch is wrong? 

> 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.

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.

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.

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.

    ;-)

> Ideally, we would turn them off globally, but we would first have to get rid of all
> *direct* sigsetjmp/setjmp usage in VLC.

Makes sense, thanks for the quick reply.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160511/8868058d/attachment.html>


More information about the vlc-devel mailing list