Re: [PATCH 5/8] Remove __P and convert to ANSI prototypes.

[Craig Howland <howland at LGSInnovations dot com>]

On 2/1/19 4:09 AM, Corinna Vinschen wrote:
> On Feb  1 07:48, Sebastian Huber wrote:
> > On 31/01/2019 20:52, Corinna Vinschen wrote:
> > > On Jan 31 12:05, Craig Howland wrote:
> > > > On 1/31/19 8:05 AM, Sebastian Huber wrote:
> > > > > From: obrien<obrien@FreeBSD.org>
> > > > >
> > > > > * Remove 'register'. (some functions had 7+ register functions...)
> > > > > * Fix SCM ID's.
> > > > > ---
> > > > >     newlib/libc/posix/scandir.c | 15 ++++++---------
> > > > >     1 file changed, 6 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/newlib/libc/posix/scandir.c b/newlib/libc/posix/scandir.c
> > > > > index 97a16cf7b..8404cd0de 100644
> > > > > --- a/newlib/libc/posix/scandir.c
> > > > > +++ b/newlib/libc/posix/scandir.c
> > > > > @@ -33,6 +33,7 @@
> > > > >     #include <sys/cdefs.h>
> > > > >     __SCCSID("@(#)scandir.c	8.3 (Berkeley) 1/2/94");
> > > > > +__FBSDID("$FreeBSD$");
> > > > >     /*
> > > > >      * Scan the directory dirname calling select to make a list of selected
> > > > > @@ -64,18 +65,14 @@ __SCCSID("@(#)scandir.c	8.3 (Berkeley) 1/2/94");
> > > > >         (offsetof (struct dirent, d_name) + ((strlen((dp)->d_name)+1 + 3) &~ 3))
> > > > >     #endif
> > > > > -#ifndef __P
> > > > > -#define __P(args) ()
> > > > > -#endif
> > > > >     int
> > > > > -scandir (const char *dirname,
> > > > > -	struct dirent ***namelist,
> > > > > -	int (*select) __P((const struct dirent *)),
> > > > > -	int (*dcomp) __P((const struct dirent **, const struct dirent **)))
> > > > > +scandir(const char *dirname, struct dirent ***namelist,
> > > > > +    int (*select)(const struct dirent *), int (*dcomp)(const struct dirent **,
> > > > > +	const struct dirent **))
> > > > >     {
> > > > > -	register struct dirent *d, *p, **names;
> > > > > -	register size_t nitems;
> > > > > +	struct dirent *d, *p, **names;
> > > > > +	size_t nitems;
> > > > >     	struct stat stb;
> > > > >     	long arraysz;
> > > > >     	DIR *dirp;
> > > >        Why?  This seems a step backwards, as the coder is giving a
> > > > recommendation to the compiler, presumably based on the coder's knowledge of
> > > > [...]
> > > I expect the compiler to use them wisely.
> > I don't really care about the register keyword. I will remove this part of
> > the patch.
> >
> > What about the __P removal?
> Looks good.
>
>
> Corinna
     I agree with Coinna that __P removal is good.  (Goes with the big ANSI 
cleanup done last year.)
     I had never done so before, but I found how GCC uses the "register" 
keyword, and it was not what I had expected.  In short (skipping some 
specialized standard-extension sub-cases), from the 7.2.0 manual, GCC uses 
register only with -O0 (putting non-"register"-qualified variables in the 
stack), ignoring it otherwise.  (I had previously assumed that when the 
optimizer was on that a register attribute would add weighting to the 
optimizer's choices.)  Given this, I think it is more important to keep them for 
those times when someone might choose to use -O0.  So I'm glad you did decide to 
remove this part of the patch.
                Craig