Add forward declarations

Discussion:

Christophe Lyon

2018-10-01 21:37:35 UTC

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

Craig Howland

2018-10-02 01:04:03 UTC

Permalink

Post by Christophe Lyon
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?

Craig

Christophe Lyon

2018-10-02 08:55:07 UTC

Permalink

Post by Craig Howland

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?

I agree with you, one of my intentions was precisely to get
knowledgeable feedback because this patch indeed looks a little bit
like a kludge :)

I've noticed that the prototypes are conditionally defined in
newlib/libc/include/sys/unistd.h
under #ifdef _COMPILING_NEWLIB
but I'm not familiar enough with newlib build system to know if this
is the file to include or not.

In particular, as I said before, the warnings appear when building for
Aarch64 and not for Arm, so different code paths are used at least by
these two targets.

Post by Craig Howland
Craig

Jeff Johnston

2018-10-02 15:31:34 UTC

Permalink

Post by Craig Howland

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-*-*.

-- Jeff J.

Post by Craig Howland
Craig

Christophe Lyon

2018-10-05 09:22:27 UTC

Permalink

Post by Jeff Johnston

Post by Craig Howland

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?

Post by Jeff Johnston
-- Jeff J.

Post by Craig Howland
Craig

Richard Earnshaw (lists)

2018-10-05 12:28:35 UTC

Permalink

Post by Christophe Lyon

Post by Jeff Johnston

Post by Craig Howland

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.

Post by Christophe Lyon

Post by Jeff Johnston
-- Jeff J.

Post by Craig Howland
Craig
newlib-5.txt
commit dbfb9670991b3b3c31b98b92f388d4235fa7b9ca
Date: Mon Oct 1 21:16:40 2018 +0000
Define _COMPILING_NEWLIB on aarch64 to define function prototypes from
unistd.h.
* 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