svn commit: r297408 - head/lib/libc/stdio

Bruce Evans brde at optusnet.com.au
Wed Mar 30 07:42:15 UTC 2016


On Wed, 30 Mar 2016, Pedro F. Giffuni wrote:

> Log:
>  freopen(3): prevent uninitialized errno.
>
>  Revert r297407 and redo it cleanly.

This might need a few more commits to do it correctly.

> Modified: head/lib/libc/stdio/freopen.c
> ==============================================================================
> --- head/lib/libc/stdio/freopen.c	Wed Mar 30 01:32:08 2016	(r297407)
> +++ head/lib/libc/stdio/freopen.c	Wed Mar 30 06:13:58 2016	(r297408)
> @@ -66,8 +66,7 @@ freopen(const char * __restrict file, co
> 		(void) fclose(fp);
> 		errno = sverrno;
> 		return (NULL);
> -	} else
> -		sverrno = 0;
> +	}

This uses the one value that is clearly wrong.  C Standard library
functions are not permitted to set errno to 0, but that is what happens
if this setting of sverrno is actually used.  Most C Standard library
functions are permitted to set errno to any nonzero value even if they
succeed.  For this particular function, errno has an unspecified value
in Standard C after the function returns, but POSIX extends this and
specifies some settings.  This extension is permitted because, unlike
for some other functions, Standard C doesn't specify anything for
errno.

It would not be wrong to set sverrno to errno here, of course without
the style bug, but any use of the value would be dubious.

>
> 	FLOCKFILE(fp);
>
> @@ -79,6 +78,7 @@ freopen(const char * __restrict file, co
> 	 * re-open the same file with a different mode. We allow this only
> 	 * if the modes are compatible.
> 	 */
> +	sverrno = 0;
> 	if (file == NULL) {
> 		/* See comment below regarding freopen() of closed files. */
> 		if (fp->_flags == 0) {

Setting it to 0 here is equivalent to setting it above, except it doesn't
have the style bug.  Actually saving errno in it is not quite eqivalent
to saving errno above, since there are functions that might set errno in
between.

It should be set above, or better not at all.  You need to understand the
function better to place it exactly correctly.  The function only restores
from sverrno after the "finish" label in the f < 0 case.  I think it
intends to restore the errno that was set for one of the 2
'f = _open(...)' expressions, so if it actually uses the above setting
then that is a bug.  A complete fix would reorganise the function to
avoid "goto finish" so that it is clear to humans and coverities that
the above setting is never used.

Bruce


More information about the svn-src-all mailing list