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

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<[email protected]>
>>>
>>> * 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
> If the developer guesses on what should be done to optimize, rather then
> letting the compiler decide what to optimize, the developer is usually
> wrong.  That's what -O2 is for, no?  On a RISC with lots of registers
> 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?

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : [email protected]
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.