flac dev - Jan 2016 - About the commit "Fix compiler warnings from new compiler flags."

https://git.xiph.org/?p=flac.git;a=commit;h=91790ef965553fd6d76e1f51b55dc4de3b54ad4b

It (among other things) changes (void)chown(...) into
FLAC_CHECK_RETURN(chown(...)).

I don't know what warning "(void)chown(...);" raises, and I cannot
test it
because I don't have Linux, but FLAC_CHECK_RETURN calls printf, and IMHO
the library function shouldn't do it. So it makes sense to revert
this part of the patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no_check_return.patch
Type: application/octet-stream
Size: 873 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20160118/37851c3d/attachment.obj

lvqcl wrote:
> I don't know what warning "(void)chown(...);" raises,
The warning is:

    metadata_iterators.c: In function ?set_file_stats_?:
    metadata_iterators.c:3362:2: warning: ignoring return value of ?chown?,
        declared with attribute warn_unused_result [-Wunused-result]
      (void)chown(filename, stats->st_uid, -1);

The reason why `chown` is declared with attribute warn_unused_result
is because a silent failure of `chown` can in some instances lead to
serious security problems.
> but FLAC_CHECK_RETURN calls printf, and IMHO
> the library function shouldn't do it.
I absolutely agree that the library should not print anything under
normal operation, but I think a failure of `chown` in this peice of
code is extremely unlikely that a) its not worth removing the printf
and b) its worthwhile informing the user.

I do however think its worth printing it to stderr instead of stdout
and I will be making that change. I'll also add a comment.

Cheers,
Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/