* [PATCH] Add missing headers to fix implicit function defns
@ 2017-01-16 0:13 Pat Pannuto
2017-01-16 9:15 ` Corinna Vinschen
2017-01-16 21:21 ` Hans-Bernhard Bröker
0 siblings, 2 replies; 6+ messages in thread
From: Pat Pannuto @ 2017-01-16 0:13 UTC (permalink / raw)
To: newlib; +Cc: Pat Pannuto
A few files were missing headers for memset/malloc, likely missed
because the files don't directly call the functions, rather they
come in via macros in libc/include/sys/reent.h:
#define _REENT_CHECK(var, what, type, size, init) do { \
struct _reent *_r = (var); \
if (_r->what == NULL) { \
_r->what = (type)malloc(size); \
#define _REENT_CHECK_ASCTIME_BUF(var) \
_REENT_CHECK(var, _asctime_buf, char *, _REENT_ASCTIME_SIZE, \
memset((var)->_asctime_buf, 0, _REENT_ASCTIME_SIZE))
Without these fixes, implicit function signatures are provided,
which gcc warns could cause aliasing issues down the line:
../../../../../../../newlib-2.5.0/newlib/libc/time/asctime.c:62:3: warning: type of 'memset' does not match original declaration [-Wlto-type-mismatch]
/Volumes/code/external/newlib-cygwin/newlib/libc/include/string.h:29:7: note: return value type mismatch
_PTR _EXFUN(memset,(_PTR, int, size_t));
^
/Volumes/code/external/newlib-cygwin/newlib/libc/include/string.h:29:7: note: 'memset' was previously declared here
/Volumes/code/external/newlib-cygwin/newlib/libc/include/string.h:29:7: note: code may be misoptimized unless -fno-strict-aliasing is used
../../../../../../../newlib-2.5.0/newlib/libc/time/asctime.c:62:3: warning: type of 'malloc' does not match original declaration [-Wlto-type-mismatch]
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: return value type mismatch
extern _PTR malloc _PARAMS ((size_t));
^
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: 'malloc' was previously declared here
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: code may be misoptimized unless -fno-strict-aliasing is used
../../../../../../../newlib-2.5.0/newlib/libc/time/lcltime.c:58:3: warning: type of 'malloc' does not match original declaration [-Wlto-type-mismatch]
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: return value type mismatch
extern _PTR malloc _PARAMS ((size_t));
^
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: 'malloc' was previously declared here
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: code may be misoptimized unless -fno-strict-aliasing is used
../../../../../../../newlib-2.5.0/newlib/libc/string/strsignal.c:70:3: warning: type of 'malloc' does not match original declaration [-Wlto-type-mismatch]
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: return value type mismatch
extern _PTR malloc _PARAMS ((size_t));
^
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: 'malloc' was previously declared here
/Volumes/code/external/newlib-cygwin/newlib/libc/include/malloc.h:37:13: note: code may be misoptimized unless -fno-strict-aliasing is used
Including the proper headers elminates the implicit function
signatures and these warnings.
---
newlib/libc/string/strsignal.c | 1 +
newlib/libc/string/strtok.c | 1 +
newlib/libc/time/asctime.c | 2 ++
newlib/libc/time/lcltime.c | 1 +
4 files changed, 5 insertions(+)
diff --git a/newlib/libc/string/strsignal.c b/newlib/libc/string/strsignal.c
index e03c1086b..94ae26db1 100644
--- a/newlib/libc/string/strsignal.c
+++ b/newlib/libc/string/strsignal.c
@@ -56,6 +56,7 @@ QUICKREF
#include <string.h>
#include <signal.h>
#include <stdio.h>
+#include <stdlib.h>
#include <reent.h>
char *
diff --git a/newlib/libc/string/strtok.c b/newlib/libc/string/strtok.c
index 21607e04a..8d07ab387 100644
--- a/newlib/libc/string/strtok.c
+++ b/newlib/libc/string/strtok.c
@@ -85,6 +85,7 @@ QUICKREF
/* undef STRICT_ANSI so that strtok_r prototype will be defined */
#undef __STRICT_ANSI__
#include <string.h>
+#include <stdlib.h>
#include <_ansi.h>
#include <reent.h>
diff --git a/newlib/libc/time/asctime.c b/newlib/libc/time/asctime.c
index 024310bbf..f56b511b8 100644
--- a/newlib/libc/time/asctime.c
+++ b/newlib/libc/time/asctime.c
@@ -47,6 +47,8 @@ ANSI C requires <<asctime>>.
<<asctime>> requires no supporting OS subroutines.
*/
+#include <stdlib.h>
+#include <string.h>
#include <time.h>
#include <_ansi.h>
#include <reent.h>
diff --git a/newlib/libc/time/lcltime.c b/newlib/libc/time/lcltime.c
index 16162bfb0..2c9a25fd7 100644
--- a/newlib/libc/time/lcltime.c
+++ b/newlib/libc/time/lcltime.c
@@ -44,6 +44,7 @@ ANSI C requires <<localtime>>.
<<localtime>> requires no supporting OS subroutines.
*/
+#include <stdlib.h>
#include <time.h>
#include <reent.h>
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add missing headers to fix implicit function defns
2017-01-16 0:13 [PATCH] Add missing headers to fix implicit function defns Pat Pannuto
@ 2017-01-16 9:15 ` Corinna Vinschen
2017-01-16 21:21 ` Hans-Bernhard Bröker
1 sibling, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2017-01-16 9:15 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 327 bytes --]
On Jan 15 19:12, Pat Pannuto wrote:
> A few files were missing headers for memset/malloc, likely missed
> because the files don't directly call the functions, rather they
> come in via macros in libc/include/sys/reent.h:
> [...]
Patch applied.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add missing headers to fix implicit function defns
2017-01-16 0:13 [PATCH] Add missing headers to fix implicit function defns Pat Pannuto
2017-01-16 9:15 ` Corinna Vinschen
@ 2017-01-16 21:21 ` Hans-Bernhard Bröker
2017-01-16 21:26 ` Pat Pannuto
2017-01-17 10:08 ` Sebastian Huber
1 sibling, 2 replies; 6+ messages in thread
From: Hans-Bernhard Bröker @ 2017-01-16 21:21 UTC (permalink / raw)
To: Pat Pannuto, newlib
Am 16.01.2017 um 01:12 schrieb Pat Pannuto:
> A few files were missing headers for memset/malloc, likely missed
> because the files don't directly call the functions, rather they
> come in via macros in libc/include/sys/reent.h:
Shouldn't then <sys/reent.h> be including <string.h> and <stdlib.h>
itself, to be done with it?
If a header defines macros which reference things provided by other
headers, it should make sure those other things are known, i.e. it
should include those other headers.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add missing headers to fix implicit function defns
2017-01-16 21:21 ` Hans-Bernhard Bröker
@ 2017-01-16 21:26 ` Pat Pannuto
2017-01-17 10:08 ` Sebastian Huber
1 sibling, 0 replies; 6+ messages in thread
From: Pat Pannuto @ 2017-01-16 21:26 UTC (permalink / raw)
To: Hans-Bernhard Bröker; +Cc: newlib
Quite possibly. This warning at the top of <sys/reent.h> made me nervous as I
don't have a handle on exactly what reent.h is doing and how it plays with other
headers:
/* WARNING: All identifiers here must begin with an underscore. This file is
included by stdio.h and others and we therefore must only use identifiers
in the namespace allotted to us. */
So I went with the more conservative change. If you have a deeper understanding
and including other std* headers in reent.h is safe, then I agree it's
probably a
better fix.
On Mon, Jan 16, 2017 at 4:21 PM, Hans-Bernhard Bröker
<HBBroeker@t-online.de> wrote:
> Am 16.01.2017 um 01:12 schrieb Pat Pannuto:
>>
>> A few files were missing headers for memset/malloc, likely missed
>> because the files don't directly call the functions, rather they
>> come in via macros in libc/include/sys/reent.h:
>
>
> Shouldn't then <sys/reent.h> be including <string.h> and <stdlib.h> itself,
> to be done with it?
>
> If a header defines macros which reference things provided by other headers,
> it should make sure those other things are known, i.e. it should include
> those other headers.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add missing headers to fix implicit function defns
2017-01-16 21:21 ` Hans-Bernhard Bröker
2017-01-16 21:26 ` Pat Pannuto
@ 2017-01-17 10:08 ` Sebastian Huber
2017-01-17 18:22 ` Hans-Bernhard Bröker
1 sibling, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2017-01-17 10:08 UTC (permalink / raw)
To: Hans-Bernhard Bröker, Pat Pannuto, newlib
On 16/01/17 22:21, Hans-Bernhard Bröker wrote:
> Am 16.01.2017 um 01:12 schrieb Pat Pannuto:
>> A few files were missing headers for memset/malloc, likely missed
>> because the files don't directly call the functions, rather they
>> come in via macros in libc/include/sys/reent.h:
>
> Shouldn't then <sys/reent.h> be including <string.h> and <stdlib.h>
> itself, to be done with it?
>
> If a header defines macros which reference things provided by other
> headers, it should make sure those other things are known, i.e. it
> should include those other headers.
No, this would lead to cyclic dependencies (<stdlib.h> includes
<sys/reent.h>) and would make <string.h> and <stdlib.h> unnecessarily
visible via unrelated standard header files.
--
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 : sebastian.huber@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add missing headers to fix implicit function defns
2017-01-17 10:08 ` Sebastian Huber
@ 2017-01-17 18:22 ` Hans-Bernhard Bröker
0 siblings, 0 replies; 6+ messages in thread
From: Hans-Bernhard Bröker @ 2017-01-17 18:22 UTC (permalink / raw)
To: Sebastian Huber, newlib
Am 17.01.2017 um 11:08 schrieb Sebastian Huber:
> On 16/01/17 22:21, Hans-Bernhard Bröker wrote:
>> Am 16.01.2017 um 01:12 schrieb Pat Pannuto:
>> If a header defines macros which reference things provided by other
>> headers, it should make sure those other things are known, i.e. it
>> should include those other headers.
> No, this would lead to cyclic dependencies (<stdlib.h> includes
> <sys/reent.h>) and would make <string.h> and <stdlib.h> unnecessarily
> visible via unrelated standard header files.
Indeed. I had found that out, too, in the meantime, but forgot to CC:
the list in the reply.
But that does mean there's already a cyclic header dependency now,
introduced by <sys/reent.h> referencing to stuff from <string.h>, while
<string.h> includes <sys/reent.h> (smae for <stdlib.h>). That should
really be avoided, if at all possible. Usually, this would mean that
the actual declarations/definitions of malloc() and memset() should be
in separate header fragments, to be included by both <sys/reent.h> and
<stdlib.h>/<string.h>, respectively.
Could this mean that many of the 274 occurences of #include <reent.h> in
the *.c files could actually be dropped, since what those sources really
need out of it is already provided indirectly by their includes of
<stdio.h>, <string.h>, etc.? In a small experimen the 4 files modified
by this patch, and the Cygwin build of newlib, removing their #include
<reent.h> didn't appear to break the build...
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-17 18:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 0:13 [PATCH] Add missing headers to fix implicit function defns Pat Pannuto
2017-01-16 9:15 ` Corinna Vinschen
2017-01-16 21:21 ` Hans-Bernhard Bröker
2017-01-16 21:26 ` Pat Pannuto
2017-01-17 10:08 ` Sebastian Huber
2017-01-17 18:22 ` Hans-Bernhard Bröker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).