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

[Craig Howland <howland at LGSInnovations dot com>]

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 the algorithm.  
register can effectively be ignored by compilers (C99 section 6.7.1), so there 
is no harm in having them. In this particular case there are 9 variables 
declared and register is put on 4 of them.  Even if you put stock into the 
general complaint of 'some functions had 7+ register functions', it is not so 
applicable to 4.
      I do not support the complaint as a general rule, anyway.  If there were 
a function with 30 variables and 8 had register, that could be fine.  7 out of 
7, perhaps then reasonable to wonder.  One of the hallmarks of RISC is lots of 
registers.  Plenty more than 7+, even, should generally be available on most 
platforms.  (32 GPRs is pretty common, minus a few for ABI purposes leaves 
perhaps a couple dozen.)  Now if this had been brought up back in 1994 when this 
file is dated, then 4 register would have been a more interesting discussion 
(M68k only had 8 GPR, etc.), but not now.
     I understand the desire to stay in sync with FreeBSD, but does that 
include bad decisions?  I suppose perhaps it should--but only with discussion.
                Craig