Re: Add forward declarations

On 05/10/18 10:22, Christophe Lyon wrote:
> On Tue, 2 Oct 2018 at 17:31, Jeff Johnston <[email protected]> wrote:
>>
>> On Mon, Oct 1, 2018 at 9:04 PM, Craig Howland <[email protected]>
>> wrote:
>>
>>> On 10/01/2018 05:37 PM, Christophe Lyon wrote:
>>>
>>>> Hi,
>>>>
>>>> While building newlib for Aarch64, I noticed several warnings because
>>>> of missing prototypes. I am not familiar enough to know why the same
>>>> warnings do not appear when building for Arm.
>>>>
>>>> This patch adds the missing prototypes, tested by rebuilding for
>>>> Aarch64 and Arm, it removed the warnings and didn't generate any
>>>> error.
>>>>
>>>> OK?
>>>>
>>>> Christophe
>>>>
>>> A primary reason for prototypes is that they are for checking, and
>>> fully-proper checking is a single header file that checks both the source
>>> providing the function as well as places that call the function.  That is,
>>> adding prototypes in the function definition file purely for the purposes
>>> of avoiding missing prototype warnings is missing the real usefulness
>>> behind the warning and giving a false sense of security.  (You'd be better
>>> off just taking -Wmissing-prototypes out of your options to avoid those
>>> particular warnings.)  The prototypes need to be added to a header file,
>>> not the functions' own source.
>>>
>>> Put another way, as a general goal I'd think we ought to be fixing the
>>> cause of a warning to remove the warning, rather than doing things to
>>> "hide" or "mask" the warning (which could be considered as kludges).
>>> Putting protoypes into the function source file is clearly masking the
>>> deficiency that there is not a header file with prototypes, rather than
>>> fixing the actual deficiency.  While there certainly are times when
>>> addressing the root cause of a warning is not possible, this does not seem
>>> to be one where it is appropriate. Probably something like sys/_syscall.h
>>> (or some such name) should be created and used.  We should be cleaning up
>>> bad practice when it exists, not furthering it.  So, definitely a good
>>> thing to get rid of the warnings, but there is a much better approach to
>>> take in this case.
>>>
>>> Corrina or Jeff, any suggestions for a good place for syscall prototypes?
>>> (My first thought was sys/syscalls.h, but the Linux syscall(2) manpage
>>> mentions that related to syscall() and "For SYS_xxx definitions."  Perhaps
>>> it would be OK, but perhaps not, too, which is why I ended up suggesting
>>> _syscall.h.)  Does someone know about a precedent in BSD or the like?
>>>
>>
>> My suggestion is to set the _COMPILING_NEWLIB flag in configure.host for
>> aarch64 as is already done for arm-*-*.
>>
> 
> OK, thanks for the suggestion, it works. Is this new patch OK?
> 

Pushed.

R.

> 
>> -- Jeff J.
>>
>>
>>> Craig
>>>
>>>
>>> newlib-5.txt
>>>
>>>
>>> commit dbfb9670991b3b3c31b98b92f388d4235fa7b9ca
>>> Author: Christophe Lyon <[email protected]>
>>> Date:   Mon Oct 1 21:16:40 2018 +0000
>>>
>>>     Define _COMPILING_NEWLIB on aarch64 to define function prototypes from
>>>     unistd.h.
>>>     
>>>     2018-10-01  Christophe Lyon  <[email protected]>
>>>     
>>>     	* newlib/configure.host: Define _COMPILING_NEWLIB for aarch64.
>>>
>>> diff --git a/newlib/configure.host b/newlib/configure.host
>>> index c5b7734..9e809c9 100644
>>> --- a/newlib/configure.host
>>> +++ b/newlib/configure.host
>>> @@ -438,6 +438,9 @@ case "${host}" in
>>>  	sys_dir=a29khif
>>>  	signal_dir=
>>>  	;;
>>> +  aarch64*-*-*)
>>> +	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
>>> +	;;
>>>    arm*-*-*)
>>>  	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
>>>  	sys_dir=arm