Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 Dec 2017 15:31:11 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Mark Johnston <markj@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r326626 - head/lib/libproc
Message-ID:  <20171207142408.F1895@besplex.bde.org>
In-Reply-To: <201712061752.vB6Hq1LZ031860@repo.freebsd.org>
References:  <201712061752.vB6Hq1LZ031860@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 6 Dec 2017, Mark Johnston wrote:

> Log:
>  Use a global extern declaration to appease gcc.

Why not do this to fix the bug which is reported by gcc as requested by using
-Wnested-externs (given by WARNS?=6 in ../Makefile.inc).  clang doesn't
suuport this option except in a broken way by ignoring it.

-Wnested-includes is turned off for CXX in sys.mk, but that doesn't apply
here.

> Modified: head/lib/libproc/proc_create.c
> ==============================================================================
> --- head/lib/libproc/proc_create.c	Wed Dec  6 17:50:10 2017	(r326625)
> +++ head/lib/libproc/proc_create.c	Wed Dec  6 17:52:01 2017	(r326626)
> @@ -47,6 +47,8 @@ __FBSDID("$FreeBSD$");
>
> #include "_libproc.h"
>
> +extern char * const *environ;
> +
> static int	getelfclass(int);
> static int	proc_init(pid_t, int, int, struct proc_handle **);
>
> @@ -179,7 +181,6 @@ int
> proc_create(const char *file, char * const *argv, char * const *envp,
>     proc_child_func *pcf, void *child_arg, struct proc_handle **pphdl)
> {
> -	extern char * const *environ;
> 	struct proc_handle *phdl;
> 	int error, status;
> 	pid_t pid;

This is still broken.  'environ' is hard to use since it is not declared
in any standard header, but it has a whole man page environ(7) whose
synoposis declares it as char **.  The type mismatch is not detected
because no header files that declares 'environ' is included (because none
exists).

'environ' is also documented in exec(3) and posix_spawn(3).

'environ' is correctly declared in all 9 other source files and 3 man
pages in /usr/src/lib/ that declare it.

Declaring it as extern in file scope is worse in some ways that
declaring it in block scope, since it breaks this warning so that the
bug of misdeclaring it is harder to notice.  WARNS=6 also enables
-Wredundant-decls to detect this bug when it is only a style bug.
Normally variables are declared in headers, so redeclaring them in .c
files is a type mismatch if the declarations are compatible (not
necessarily the same) and a style bug if they are compatible.  gcc up
to least 4.2.1] has some bugs involving it not understanding redundancy.
clang silently ignores this option too.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171207142408.F1895>