mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH] Include missing reference to stdio.h for EOF definition
@ 2016-02-23 22:49 Michael Meeuwisse
  2016-02-25  9:50 ` Michael Meeuwisse
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Meeuwisse @ 2016-02-23 22:49 UTC (permalink / raw)
  To: musl; +Cc: Michael Meeuwisse

No binary changes as it was already included implicitly;
. src/multibyte/internal.h
.. ./src/internal/locale_impl.h
... ./src/internal/libc.h
.... ./include/stdio.h

However, all evidence suggest this to be a fortunate accident rather than an intentional design choice.

Signed-off-by: Michael Meeuwisse <mickeymeeuw+git@gmail.com>
---
 src/multibyte/wctob.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/multibyte/wctob.c b/src/multibyte/wctob.c
index 4aeda6a..b484a3f 100644
--- a/src/multibyte/wctob.c
+++ b/src/multibyte/wctob.c
@@ -1,4 +1,5 @@
 #include <wchar.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include "internal.h"
 
-- 
2.5.0



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Include missing reference to stdio.h for EOF definition
  2016-02-23 22:49 [PATCH] Include missing reference to stdio.h for EOF definition Michael Meeuwisse
@ 2016-02-25  9:50 ` Michael Meeuwisse
  2016-02-25 18:27   ` Rich Felker
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Meeuwisse @ 2016-02-25  9:50 UTC (permalink / raw)
  To: musl; +Cc: Michael Meeuwisse

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

Following up. I know this is a rather silly patch to accept, but I'm trying
to get OSv (http://osv.io) up to date. It currently uses 0.9.12 which did
have this line, but it was dropped (imho erroneously) last June in
following commit;
http://git.musl-libc.org/cgit/musl/commit/?id=1507ebf837334e9e07cfab1ca1c2e88449069a80

Since OSv does a few.. unconventional.. things in regards to musl
inclusion, relying on stdio.h being included implicitly here doesn't work,
and working around this omission is a far larger effort than simply
correcting it upstream. Not saying I'm not trying to make the inclusion
more conventional, just that there's bigger fish to fry at the moment.

Thank you,


Michael


On Tue, Feb 23, 2016 at 11:49 PM, Michael Meeuwisse <mickeymeeuw@gmail.com>
wrote:

> No binary changes as it was already included implicitly;
> . src/multibyte/internal.h
> .. ./src/internal/locale_impl.h
> ... ./src/internal/libc.h
> .... ./include/stdio.h
>
> However, all evidence suggest this to be a fortunate accident rather than
> an intentional design choice.
>
> Signed-off-by: Michael Meeuwisse <mickeymeeuw+git@gmail.com>
> ---
>  src/multibyte/wctob.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/multibyte/wctob.c b/src/multibyte/wctob.c
> index 4aeda6a..b484a3f 100644
> --- a/src/multibyte/wctob.c
> +++ b/src/multibyte/wctob.c
> @@ -1,4 +1,5 @@
>  #include <wchar.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include "internal.h"
>
> --
> 2.5.0
>
>

[-- Attachment #2: Type: text/html, Size: 2413 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Re: [PATCH] Include missing reference to stdio.h for EOF definition
  2016-02-25  9:50 ` Michael Meeuwisse
@ 2016-02-25 18:27   ` Rich Felker
  0 siblings, 0 replies; 3+ messages in thread
From: Rich Felker @ 2016-02-25 18:27 UTC (permalink / raw)
  To: musl

On Thu, Feb 25, 2016 at 10:50:01AM +0100, Michael Meeuwisse wrote:
> Following up. I know this is a rather silly patch to accept, but I'm trying
> to get OSv (http://osv.io) up to date. It currently uses 0.9.12 which did
> have this line, but it was dropped (imho erroneously) last June in
> following commit;
> http://git.musl-libc.org/cgit/musl/commit/?id=1507ebf837334e9e07cfab1ca1c2e88449069a80
> 
> Since OSv does a few.. unconventional.. things in regards to musl
> inclusion, relying on stdio.h being included implicitly here doesn't work,
> and working around this omission is a far larger effort than simply
> correcting it upstream. Not saying I'm not trying to make the inclusion
> more conventional, just that there's bigger fish to fry at the moment.

Thanks for the explanation of your motivation. I wasn't opposed to
this patch but was curious why it mattered. I'd actually like to go
through the source at some point and find places we're making
unreasonable assumptions about implicit inclusion, and also get rid of
some of the unnecessary implicit inclusions (e.g. stdio.h from libc.h
makes no sense and is probably just there because __lockfile is
declared in a poorly chosen place). But for now I'm happy to accept
this patch as a start.

Rich



> On Tue, Feb 23, 2016 at 11:49 PM, Michael Meeuwisse <mickeymeeuw@gmail.com>
> wrote:
> 
> > No binary changes as it was already included implicitly;
> > . src/multibyte/internal.h
> > .. ./src/internal/locale_impl.h
> > ... ./src/internal/libc.h
> > .... ./include/stdio.h
> >
> > However, all evidence suggest this to be a fortunate accident rather than
> > an intentional design choice.
> >
> > Signed-off-by: Michael Meeuwisse <mickeymeeuw+git@gmail.com>
> > ---
> >  src/multibyte/wctob.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/multibyte/wctob.c b/src/multibyte/wctob.c
> > index 4aeda6a..b484a3f 100644
> > --- a/src/multibyte/wctob.c
> > +++ b/src/multibyte/wctob.c
> > @@ -1,4 +1,5 @@
> >  #include <wchar.h>
> > +#include <stdio.h>
> >  #include <stdlib.h>
> >  #include "internal.h"
> >
> > --
> > 2.5.0
> >
> >


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-25 18:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23 22:49 [PATCH] Include missing reference to stdio.h for EOF definition Michael Meeuwisse
2016-02-25  9:50 ` Michael Meeuwisse
2016-02-25 18:27   ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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 NNTP newsgroup(s).