* [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately
2012-10-20 20:15 [PATCH 0/4] Import <netinet/ether.h> features from NetBSD Abdoulaye Walsimou Gaye
@ 2012-10-20 20:15 ` Abdoulaye Walsimou Gaye
2012-10-20 23:49 ` Rich Felker
2012-10-20 20:15 ` [PATCH 2/4] Add basic sys/cdefs.h found on most unix Abdoulaye Walsimou Gaye
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou Gaye @ 2012-10-20 20:15 UTC (permalink / raw)
To: musl; +Cc: Abdoulaye Walsimou Gaye
This will allow toolchain build systems using 3-stages gcc build
to not fail at second stage (which needs lib/crt*.o file).
Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index 3c55c8f..ee9f14d 100644
--- a/Makefile
+++ b/Makefile
@@ -139,6 +139,7 @@ install-headers: $(ALL_INCLUDES:include/%=$(DESTDIR)$(includedir)/%)
install-tools: $(ALL_TOOLS:tools/%=$(DESTDIR)$(bindir)/%)
+install-startfiles: $(CRT_LIBS:lib/%=$(DESTDIR)$(libdir)/%)
.PRECIOUS: $(CRT_LIBS:lib/%=crt/%)
--
1.7.9.5
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately
2012-10-20 20:15 ` [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately Abdoulaye Walsimou Gaye
@ 2012-10-20 23:49 ` Rich Felker
0 siblings, 0 replies; 27+ messages in thread
From: Rich Felker @ 2012-10-20 23:49 UTC (permalink / raw)
To: musl
On Sat, Oct 20, 2012 at 10:15:42PM +0200, Abdoulaye Walsimou Gaye wrote:
> This will allow toolchain build systems using 3-stages gcc build
> to not fail at second stage (which needs lib/crt*.o file).
Could you explain what you mean? If a gcc for the target exists at
this stage, then it should be possible to build the entire libc. If it
doesn't exist, then the Makefile will not work to build the start
files, even though it would be possible to build them by invoking the
assembler directly.
I'm not opposed to this patch but I'd like to understand how it helps.
> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> ---
> Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 3c55c8f..ee9f14d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -139,6 +139,7 @@ install-headers: $(ALL_INCLUDES:include/%=$(DESTDIR)$(includedir)/%)
>
> install-tools: $(ALL_TOOLS:tools/%=$(DESTDIR)$(bindir)/%)
>
> +install-startfiles: $(CRT_LIBS:lib/%=$(DESTDIR)$(libdir)/%)
If this patch is adopted, this target should be added to .PHONY and I
think the duplicate rule under install-libs should be replaced with a
dependency on install-startfiles, no?
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 20:15 [PATCH 0/4] Import <netinet/ether.h> features from NetBSD Abdoulaye Walsimou Gaye
2012-10-20 20:15 ` [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately Abdoulaye Walsimou Gaye
@ 2012-10-20 20:15 ` Abdoulaye Walsimou Gaye
2012-10-20 23:18 ` Isaac Dunham
2012-10-20 20:15 ` [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD Abdoulaye Walsimou Gaye
2012-10-20 20:15 ` [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r() Abdoulaye Walsimou Gaye
3 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou Gaye @ 2012-10-20 20:15 UTC (permalink / raw)
To: musl; +Cc: Abdoulaye Walsimou Gaye
Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
---
include/sys/cdefs.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 include/sys/cdefs.h
diff --git a/include/sys/cdefs.h b/include/sys/cdefs.h
new file mode 100644
index 0000000..917cdd6
--- /dev/null
+++ b/include/sys/cdefs.h
@@ -0,0 +1,22 @@
+#ifndef _SYS_CDEFS_H_
+#define _SYS_CDEFS_H_
+
+#if defined(__cplusplus)
+#define __BEGIN_DECLS extern "C" {
+#define __END_DECLS }
+#else
+#define __BEGIN_DECLS
+#define __END_DECLS
+#endif
+
+/*
+ * Test if currently used gcc version is equal to or greater than specified.
+ */
+#if defined(__GNUC__)
+#define __GNUC_PREREQ(__maj, __min) \
+ (__GNUC__ > (__maj) || __GNUC__ == (__maj) && __GNUC_MINOR__ >= (__min))
+#else
+#define __GNUC_PREREQ(__maj, __min) 0
+#endif
+
+#endif /* !_SYS_CDEFS_H_ */
--
1.7.9.5
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 20:15 ` [PATCH 2/4] Add basic sys/cdefs.h found on most unix Abdoulaye Walsimou Gaye
@ 2012-10-20 23:18 ` Isaac Dunham
2012-10-20 23:38 ` Abdoulaye Walsimou GAYE
0 siblings, 1 reply; 27+ messages in thread
From: Isaac Dunham @ 2012-10-20 23:18 UTC (permalink / raw)
To: musl
On Sat, 20 Oct 2012 22:15:43 +0200
Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> ---
> include/sys/cdefs.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 include/sys/cdefs.h
I'm pretty sure that the last three times sys/cdefs.h was proposed, it was rejected.
> diff --git a/include/sys/cdefs.h b/include/sys/cdefs.h
> new file mode 100644
> index 0000000..917cdd6
> --- /dev/null
> +++ b/include/sys/cdefs.h
> @@ -0,0 +1,22 @@
> +#ifndef _SYS_CDEFS_H_
> +#define _SYS_CDEFS_H_
> +
> +#if defined(__cplusplus)
> +#define __BEGIN_DECLS extern "C" {
> +#define __END_DECLS }
> +#else
> +#define __BEGIN_DECLS
> +#define __END_DECLS
> +#endif
> +
> +/*
> + * Test if currently used gcc version is equal to or greater than specified.
> + */
> +#if defined(__GNUC__)
> +#define __GNUC_PREREQ(__maj, __min) \
> + (__GNUC__ > (__maj) || __GNUC__ == (__maj) && __GNUC_MINOR__ >= (__min))
> +#else
> +#define __GNUC_PREREQ(__maj, __min) 0
> +#endif
> +
> +#endif /* !_SYS_CDEFS_H_ */
> --
--
Isaac Dunham <idunham@lavabit.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 23:18 ` Isaac Dunham
@ 2012-10-20 23:38 ` Abdoulaye Walsimou GAYE
2012-10-20 23:38 ` Rich Felker
2012-10-20 23:50 ` Isaac Dunham
0 siblings, 2 replies; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-20 23:38 UTC (permalink / raw)
To: musl
On 10/21/2012 01:18 AM, Isaac Dunham wrote:
> On Sat, 20 Oct 2012 22:15:43 +0200
> Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
>
>> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
>> ---
>> include/sys/cdefs.h | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>> create mode 100644 include/sys/cdefs.h
> I'm pretty sure that the last three times sys/cdefs.h was proposed, it was rejected.
>
>
Unfortunately many packages (wrongly?) use to rely on macros defined there,
sometimes indirectly via <features.h>.
Cheers,
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 23:38 ` Abdoulaye Walsimou GAYE
@ 2012-10-20 23:38 ` Rich Felker
2012-10-21 0:13 ` Abdoulaye Walsimou GAYE
2012-10-20 23:50 ` Isaac Dunham
1 sibling, 1 reply; 27+ messages in thread
From: Rich Felker @ 2012-10-20 23:38 UTC (permalink / raw)
To: musl
On Sun, Oct 21, 2012 at 01:38:52AM +0200, Abdoulaye Walsimou GAYE wrote:
> On 10/21/2012 01:18 AM, Isaac Dunham wrote:
> >On Sat, 20 Oct 2012 22:15:43 +0200
> >Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
> >
> >>Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> >>---
> >> include/sys/cdefs.h | 22 ++++++++++++++++++++++
> >> 1 file changed, 22 insertions(+)
> >> create mode 100644 include/sys/cdefs.h
> >I'm pretty sure that the last three times sys/cdefs.h was proposed,
> >it was rejected.
>
> Unfortunately many packages (wrongly?) use to rely on macros defined there,
I've found it's really very few; an equivalent sys/cdefs.h does not
exist on most systems. It was never intended for use by applications;
it's an internal part of glibc (and perhaps also some BSDs?) used for
handling backwardsness like pre-ANSI C compilers (abstracting const
away as __const, or abstracting away prototypes with __P()) and
optional use of GCC-specific features.
For things like 'extern "C"', there's no reason to use sys/cdefs.h;
the just writing the code it expands to inline is much more
clear/informative and provides better performance as a nice side
effect.
> sometimes indirectly via <features.h>.
I don't see what you mean by this.
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 23:38 ` Rich Felker
@ 2012-10-21 0:13 ` Abdoulaye Walsimou GAYE
2012-10-21 0:11 ` Rich Felker
0 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-21 0:13 UTC (permalink / raw)
To: musl
On 10/21/2012 01:38 AM, Rich Felker wrote:
> On Sun, Oct 21, 2012 at 01:38:52AM +0200, Abdoulaye Walsimou GAYE wrote:
>> On 10/21/2012 01:18 AM, Isaac Dunham wrote:
>>> On Sat, 20 Oct 2012 22:15:43 +0200
>>> Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
>>>
>>>> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
>>>> ---
>>>> include/sys/cdefs.h | 22 ++++++++++++++++++++++
>>>> 1 file changed, 22 insertions(+)
>>>> create mode 100644 include/sys/cdefs.h
>>> I'm pretty sure that the last three times sys/cdefs.h was proposed,
>>> it was rejected.
>> Unfortunately many packages (wrongly?) use to rely on macros defined there,
> I've found it's really very few; an equivalent sys/cdefs.h does not
> exist on most systems. It was never intended for use by applications;
> it's an internal part of glibc (and perhaps also some BSDs?) used for
> handling backwardsness like pre-ANSI C compilers (abstracting const
> away as __const, or abstracting away prototypes with __P()) and
> optional use of GCC-specific features.
But applications borrowed from systems internal and ported to others
systems tend to use these macros (libtirpc, libbsd come in mind).
Major BSD systems have it (FreeBSD, NetBSD, OpenBSD, PCBSD), it not
a reason to have it on linux systems but it helps/simplifies packages porting
from these OS.
>
> For things like 'extern "C"', there's no reason to use sys/cdefs.h;
> the just writing the code it expands to inline is much more
> clear/informative and provides better performance as a nice side
> effect.
>
>> sometimes indirectly via <features.h>.
> I don't see what you mean by this.
>
> Rich
some applications use <features.h> to get macros defined in <sys/cdefs.h>
as on glibc, eglibc, uClibc <features.h> have a #include <sys/cdefs.h>
cheers,
AWG
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-21 0:13 ` Abdoulaye Walsimou GAYE
@ 2012-10-21 0:11 ` Rich Felker
2012-10-21 0:38 ` Abdoulaye Walsimou GAYE
0 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2012-10-21 0:11 UTC (permalink / raw)
To: musl
On Sun, Oct 21, 2012 at 02:13:47AM +0200, Abdoulaye Walsimou GAYE wrote:
> On 10/21/2012 01:38 AM, Rich Felker wrote:
> >On Sun, Oct 21, 2012 at 01:38:52AM +0200, Abdoulaye Walsimou GAYE wrote:
> >>On 10/21/2012 01:18 AM, Isaac Dunham wrote:
> >>>On Sat, 20 Oct 2012 22:15:43 +0200
> >>>Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
> >>>
> >>>>Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> >>>>---
> >>>> include/sys/cdefs.h | 22 ++++++++++++++++++++++
> >>>> 1 file changed, 22 insertions(+)
> >>>> create mode 100644 include/sys/cdefs.h
> >>>I'm pretty sure that the last three times sys/cdefs.h was proposed,
> >>>it was rejected.
> >>Unfortunately many packages (wrongly?) use to rely on macros defined there,
> >I've found it's really very few; an equivalent sys/cdefs.h does not
> >exist on most systems. It was never intended for use by applications;
> >it's an internal part of glibc (and perhaps also some BSDs?) used for
> >handling backwardsness like pre-ANSI C compilers (abstracting const
> >away as __const, or abstracting away prototypes with __P()) and
> >optional use of GCC-specific features.
>
> But applications borrowed from systems internal and ported to others
> systems tend to use these macros (libtirpc, libbsd come in mind).
> Major BSD systems have it (FreeBSD, NetBSD, OpenBSD, PCBSD), it not
> a reason to have it on linux systems but it helps/simplifies packages porting
> from these OS.
As it stands, these libraries/apps won't work anywhere but GNU/Linux
(by "GNU/" I mean glibc-based) or BSD. If the offending code is
removed and replaced with what should be there, they'd be a lot more
portable. So I would not say sys/cdefs.h aids in porting them; I'd say
its presence gives these libs/apps a way to be lazy and
non-portable...
> >For things like 'extern "C"', there's no reason to use sys/cdefs.h;
> >the just writing the code it expands to inline is much more
> >clear/informative and provides better performance as a nice side
> >effect.
> >
> >>sometimes indirectly via <features.h>.
> >I don't see what you mean by this.
>
> some applications use <features.h> to get macros defined in <sys/cdefs.h>
> as on glibc, eglibc, uClibc <features.h> have a #include <sys/cdefs.h>
Both of these usages are incorrect and could easily be fixed (both are
implementation-internal headers).
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-21 0:11 ` Rich Felker
@ 2012-10-21 0:38 ` Abdoulaye Walsimou GAYE
2012-10-21 0:39 ` John Spencer
0 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-21 0:38 UTC (permalink / raw)
To: musl
On 10/21/2012 02:11 AM, Rich Felker wrote:
> On Sun, Oct 21, 2012 at 02:13:47AM +0200, Abdoulaye Walsimou GAYE wrote:
>> On 10/21/2012 01:38 AM, Rich Felker wrote:
>>> On Sun, Oct 21, 2012 at 01:38:52AM +0200, Abdoulaye Walsimou GAYE wrote:
>>>> On 10/21/2012 01:18 AM, Isaac Dunham wrote:
>>>>> On Sat, 20 Oct 2012 22:15:43 +0200
>>>>> Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
>>>>>
>>>>>> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
>>>>>> ---
>>>>>> include/sys/cdefs.h | 22 ++++++++++++++++++++++
>>>>>> 1 file changed, 22 insertions(+)
>>>>>> create mode 100644 include/sys/cdefs.h
>>>>> I'm pretty sure that the last three times sys/cdefs.h was proposed,
>>>>> it was rejected.
>>>> Unfortunately many packages (wrongly?) use to rely on macros defined there,
>>> I've found it's really very few; an equivalent sys/cdefs.h does not
>>> exist on most systems. It was never intended for use by applications;
>>> it's an internal part of glibc (and perhaps also some BSDs?) used for
>>> handling backwardsness like pre-ANSI C compilers (abstracting const
>>> away as __const, or abstracting away prototypes with __P()) and
>>> optional use of GCC-specific features.
>> But applications borrowed from systems internal and ported to others
>> systems tend to use these macros (libtirpc, libbsd come in mind).
>> Major BSD systems have it (FreeBSD, NetBSD, OpenBSD, PCBSD), it not
>> a reason to have it on linux systems but it helps/simplifies packages porting
>> from these OS.
> As it stands, these libraries/apps won't work anywhere but GNU/Linux
> (by "GNU/" I mean glibc-based) or BSD. If the offending code is
> removed and replaced with what should be there, they'd be a lot more
> portable. So I would not say sys/cdefs.h aids in porting them; I'd say
> its presence gives these libs/apps a way to be lazy and
> non-portable...
I am not the writer of these applications and going to patch old applications
that are there for a while is just not an option.
>
>>> For things like 'extern "C"', there's no reason to use sys/cdefs.h;
>>> the just writing the code it expands to inline is much more
>>> clear/informative and provides better performance as a nice side
>>> effect.
>>>
>>>> sometimes indirectly via <features.h>.
>>> I don't see what you mean by this.
>> some applications use <features.h> to get macros defined in <sys/cdefs.h>
>> as on glibc, eglibc, uClibc <features.h> have a #include <sys/cdefs.h>
> Both of these usages are incorrect and could easily be fixed (both are
> implementation-internal headers).
>
> Rich
May be it is incorrect, unfortunately it there for a while and some applications
rely on it.
cheers,
AWG
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-21 0:38 ` Abdoulaye Walsimou GAYE
@ 2012-10-21 0:39 ` John Spencer
2012-10-21 1:21 ` John Spencer
0 siblings, 1 reply; 27+ messages in thread
From: John Spencer @ 2012-10-21 0:39 UTC (permalink / raw)
To: musl
On 10/21/2012 02:38 AM, Abdoulaye Walsimou GAYE wrote:
> On 10/21/2012 02:11 AM, Rich Felker wrote:
>> As it stands, these libraries/apps won't work anywhere but GNU/Linux
>> (by "GNU/" I mean glibc-based) or BSD. If the offending code is
>> removed and replaced with what should be there, they'd be a lot more
>> portable. So I would not say sys/cdefs.h aids in porting them; I'd say
>> its presence gives these libs/apps a way to be lazy and
>> non-portable...
>
> I am not the writer of these applications and going to patch old
> applications
> that are there for a while is just not an option.
why not ?
>
>
> May be it is incorrect, unfortunately it there for a while and some
> applications
> rely on it.
>
and can be patched easily, as sabotage does for example.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-21 0:39 ` John Spencer
@ 2012-10-21 1:21 ` John Spencer
0 siblings, 0 replies; 27+ messages in thread
From: John Spencer @ 2012-10-21 1:21 UTC (permalink / raw)
To: musl
On 10/21/2012 02:39 AM, John Spencer wrote:
> On 10/21/2012 02:38 AM, Abdoulaye Walsimou GAYE wrote:
>> On 10/21/2012 02:11 AM, Rich Felker wrote:
>>> As it stands, these libraries/apps won't work anywhere but GNU/Linux
>>> (by "GNU/" I mean glibc-based) or BSD. If the offending code is
>>> removed and replaced with what should be there, they'd be a lot more
>>> portable. So I would not say sys/cdefs.h aids in porting them; I'd say
>>> its presence gives these libs/apps a way to be lazy and
>>> non-portable...
>>
>> I am not the writer of these applications and going to patch old
>> applications
>> that are there for a while is just not an option.
>
> why not ?
>>
>>
>> May be it is incorrect, unfortunately it there for a while and some
>> applications
>> rely on it.
>>
> and can be patched easily, as sabotage does for example.
>
>
from sabotage's 250 pkgs, only 2 needed patching:
pkg/attr:sed -i 's@__BEGIN_DECLS@@g' include/xattr.h
pkg/elfutils: "s@__BEGIN_DECLS@#ifdef __cplusplus\nextern \"C\"
{\n#endif@" \
(and the latter is not even in use anymore, since it is full of ugly
glibc-specific code, i replaced it with libelf-compat, a cleaned up version)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 23:38 ` Abdoulaye Walsimou GAYE
2012-10-20 23:38 ` Rich Felker
@ 2012-10-20 23:50 ` Isaac Dunham
2012-10-20 23:44 ` Rich Felker
1 sibling, 1 reply; 27+ messages in thread
From: Isaac Dunham @ 2012-10-20 23:50 UTC (permalink / raw)
To: musl
On Sun, 21 Oct 2012 01:38:52 +0200
Abdoulaye Walsimou GAYE <awg@embtoolkit.org> wrote:
> On 10/21/2012 01:18 AM, Isaac Dunham wrote:
> > On Sat, 20 Oct 2012 22:15:43 +0200
> > Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
> >
> >> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> >> ---
> >> include/sys/cdefs.h | 22 ++++++++++++++++++++++
> >> 1 file changed, 22 insertions(+)
> >> create mode 100644 include/sys/cdefs.h
> > I'm pretty sure that the last three times sys/cdefs.h was proposed, it was rejected.
> Unfortunately many packages (wrongly?) use to rely on macros defined there,
> sometimes indirectly via <features.h>.
Yes, but that wasn't enough of a reason the last three times.
It isn't standard (-> not universal/portable), is very easily replaced, and even if it is added for compatability, it should not be used internally.
I would say that the last point is the most important.
Isaac Dunham <idunham@lavabit.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix
2012-10-20 23:50 ` Isaac Dunham
@ 2012-10-20 23:44 ` Rich Felker
0 siblings, 0 replies; 27+ messages in thread
From: Rich Felker @ 2012-10-20 23:44 UTC (permalink / raw)
To: musl
On Sat, Oct 20, 2012 at 04:50:13PM -0700, Isaac Dunham wrote:
> On Sun, 21 Oct 2012 01:38:52 +0200
> Abdoulaye Walsimou GAYE <awg@embtoolkit.org> wrote:
>
> > On 10/21/2012 01:18 AM, Isaac Dunham wrote:
> > > On Sat, 20 Oct 2012 22:15:43 +0200
> > > Abdoulaye Walsimou Gaye <awg@embtoolkit.org> wrote:
> > >
> > >> Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
> > >> ---
> > >> include/sys/cdefs.h | 22 ++++++++++++++++++++++
> > >> 1 file changed, 22 insertions(+)
> > >> create mode 100644 include/sys/cdefs.h
> > > I'm pretty sure that the last three times sys/cdefs.h was proposed, it was rejected.
>
> > Unfortunately many packages (wrongly?) use to rely on macros defined there,
> > sometimes indirectly via <features.h>.
> Yes, but that wasn't enough of a reason the last three times.
>
> It isn't standard (-> not universal/portable), is very easily
> replaced, and even if it is added for compatability, it should not
> be used internally.
Not only that; all its macros are explicitly in the namespace reserved
for the implementation, because it's designed to be an internal part
of the implementation of libc's headers, not a public interface.
> I would say that the last point is the most important.
Yes. Even if we eventually do find a compelling reason to offer this
header, there's no reason it should be used internally. Using it is
just more expensive and obfuscated than writing things out explicitly.
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-20 20:15 [PATCH 0/4] Import <netinet/ether.h> features from NetBSD Abdoulaye Walsimou Gaye
2012-10-20 20:15 ` [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately Abdoulaye Walsimou Gaye
2012-10-20 20:15 ` [PATCH 2/4] Add basic sys/cdefs.h found on most unix Abdoulaye Walsimou Gaye
@ 2012-10-20 20:15 ` Abdoulaye Walsimou Gaye
2012-10-20 23:37 ` idunham
2012-10-21 0:42 ` John Spencer
2012-10-20 20:15 ` [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r() Abdoulaye Walsimou Gaye
3 siblings, 2 replies; 27+ messages in thread
From: Abdoulaye Walsimou Gaye @ 2012-10-20 20:15 UTC (permalink / raw)
To: musl; +Cc: Abdoulaye Walsimou Gaye
Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
---
include/netinet/ether.h | 14 ++++
src/network/ethers.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 194 insertions(+)
create mode 100644 include/netinet/ether.h
create mode 100644 src/network/ethers.c
diff --git a/include/netinet/ether.h b/include/netinet/ether.h
new file mode 100644
index 0000000..44c614e
--- /dev/null
+++ b/include/netinet/ether.h
@@ -0,0 +1,14 @@
+#ifndef _NETINET_ETHER_H
+#define _NETINET_ETHER_H
+
+#include <sys/cdefs.h>
+
+__BEGIN_DECLS
+char *ether_ntoa(const struct ether_addr *);
+struct ether_addr *ether_aton(const char *);
+int ether_ntohost(char *, const struct ether_addr *);
+int ether_hostton(const char *, struct ether_addr *);
+int ether_line(const char *, struct ether_addr *, char *);
+__END_DECLS
+
+#endif /* !_NETINET_ETHER_H */
diff --git a/src/network/ethers.c b/src/network/ethers.c
new file mode 100644
index 0000000..8014581
--- /dev/null
+++ b/src/network/ethers.c
@@ -0,0 +1,180 @@
+/* Origin NetBSD: src/lib/libc/net/ethers.c */
+
+/*
+ * ethers(3N) a la Sun.
+ *
+ * Written by Roland McGrath <roland@frob.com> 10/14/93.
+ * Public domain.
+ *
+ * port for musl by Abdoulaye Walsimou GAYE <awg@embtoolkit.org> 2012/10/15
+ */
+
+#define _BSD_SOURCE
+#include <net/ethernet.h>
+#include <netinet/ether.h>
+
+#include <sys/param.h>
+#include <assert.h>
+#include <errno.h>
+#include <paths.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#ifndef _PATH_ETHERS
+#define _PATH_ETHERS "/etc/ethers"
+#endif
+
+/*
+ * ether_ntoa():
+ * This function converts this structure into an ASCII string of the form
+ * ``xx:xx:xx:xx:xx:xx'', consisting of 6 hexadecimal numbers separated
+ * by colons. It returns a pointer to a static buffer that is reused for
+ * each call.
+ */
+char *ether_ntoa(const struct ether_addr *e)
+{
+ static char a[18];
+
+ assert(e != NULL);
+
+ (void) snprintf(a, sizeof a, "%02x:%02x:%02x:%02x:%02x:%02x",
+ e->ether_addr_octet[0], e->ether_addr_octet[1],
+ e->ether_addr_octet[2], e->ether_addr_octet[3],
+ e->ether_addr_octet[4], e->ether_addr_octet[5]);
+ return a;
+}
+
+/*
+ * ether_aton():
+ * This function converts an ASCII string of the same form and to a structure
+ * containing the 6 octets of the address. It returns a pointer to a
+ * static structure that is reused for each call.
+ */
+struct ether_addr *ether_aton(const char *s)
+{
+ static struct ether_addr n;
+ unsigned int i[6];
+
+ assert(s != NULL);
+
+ if (sscanf(s, " %x:%x:%x:%x:%x:%x ", &i[0], &i[1],
+ &i[2], &i[3], &i[4], &i[5]) == 6) {
+ n.ether_addr_octet[0] = (unsigned char)i[0];
+ n.ether_addr_octet[1] = (unsigned char)i[1];
+ n.ether_addr_octet[2] = (unsigned char)i[2];
+ n.ether_addr_octet[3] = (unsigned char)i[3];
+ n.ether_addr_octet[4] = (unsigned char)i[4];
+ n.ether_addr_octet[5] = (unsigned char)i[5];
+ return &n;
+ }
+ return NULL;
+}
+
+/*
+ * ether_ntohost():
+ * This function interrogates the data base mapping host names to Ethernet
+ * addresses, /etc/ethers.
+ * It looks up the given Ethernet address and writes the associated host name
+ * into the character buffer passed.
+ * It returns zero if it finds the requested host name and -1 if not.
+ */
+int ether_ntohost(char *hostname, const struct ether_addr *e)
+{
+ FILE *f;
+ char *p;
+ size_t len;
+ struct ether_addr try;
+
+ assert(hostname != NULL);
+ assert(e != NULL);
+
+ f = fopen(_PATH_ETHERS, "r");
+ if (f == NULL)
+ return -1;
+ while ((p = fgetln(f, &len)) != NULL) {
+ if (p[len - 1] != '\n')
+ continue; /* skip lines w/o \n */
+ p[--len] = '\0';
+ if (ether_line(p, &try, hostname) == 0 &&
+ memcmp(&try, e, sizeof try) == 0) {
+ (void)fclose(f);
+ return 0;
+ }
+ }
+ (void)fclose(f);
+ errno = ENOENT;
+ return -1;
+}
+
+/*
+ * ether_hostton():
+ * This function interrogates the data base mapping host names to Ethernet
+ * addresses, /etc/ethers.
+ * It looks up the given host name and writes the associated Ethernet address
+ * into the structure passed.
+ * It returns zero if it finds the requested address and -1 if not.
+ */
+int ether_hostton(const char *hostname, struct ether_addr *e)
+{
+ FILE *f;
+ char *p;
+ size_t len;
+ char try[MAXHOSTNAMELEN + 1];
+
+ assert(hostname != NULL);
+ assert(e != NULL);
+
+ f = fopen(_PATH_ETHERS, "r");
+ if (f==NULL)
+ return -1;
+
+ while ((p = fgetln(f, &len)) != NULL) {
+ if (p[len - 1] != '\n')
+ continue; /* skip lines w/o \n */
+ p[--len] = '\0';
+ if (ether_line(p, e, try) == 0 && strcmp(hostname, try) == 0) {
+ (void)fclose(f);
+ return 0;
+ }
+ }
+ (void)fclose(f);
+ errno = ENOENT;
+ return -1;
+}
+
+/*
+ * ether_line():
+ * This function parses a line from the /etc/ethers file and fills in the passed
+ * ``struct ether_addr'' and character buffer with the Ethernet address and host
+ * name on the line.
+ * It returns zero if the line was successfully parsed and -1 if not.
+ */
+int ether_line(const char *l, struct ether_addr *e, char *hostname)
+{
+ unsigned int i[6];
+
+#define S2(arg) #arg
+#define S1(arg) S2(arg)
+ static const char fmt[] = " %x:%x:%x:%x:%x:%x"
+ " %" S1(MAXHOSTNAMELEN) "s\n";
+#undef S2
+#undef S1
+
+ assert(l != NULL);
+ assert(e != NULL);
+ assert(hostname != NULL);
+
+ if (sscanf(l, fmt,
+ &i[0], &i[1], &i[2], &i[3], &i[4], &i[5], hostname) == 7) {
+ e->ether_addr_octet[0] = (unsigned char)i[0];
+ e->ether_addr_octet[1] = (unsigned char)i[1];
+ e->ether_addr_octet[2] = (unsigned char)i[2];
+ e->ether_addr_octet[3] = (unsigned char)i[3];
+ e->ether_addr_octet[4] = (unsigned char)i[4];
+ e->ether_addr_octet[5] = (unsigned char)i[5];
+ return 0;
+ }
+ errno = EINVAL;
+ return -1;
+}
--
1.7.9.5
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-20 20:15 ` [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD Abdoulaye Walsimou Gaye
@ 2012-10-20 23:37 ` idunham
2012-10-21 0:40 ` Abdoulaye Walsimou GAYE
2012-10-21 0:42 ` John Spencer
1 sibling, 1 reply; 27+ messages in thread
From: idunham @ 2012-10-20 23:37 UTC (permalink / raw)
To: musl
> +#ifndef _NETINET_ETHER_H
> +#define _NETINET_ETHER_H
> +
> +#include <sys/cdefs.h>
> +__BEGIN_DECLS
musl does not do nested includes unless *necessary*, nor do we define
macros unless it simplifies the code significantly.
This detail speeds up compilation significantly.
#ifdef __cplusplus
extern "C" {
#endif
is NOT something that obscures the code, and it certainly isn't worth
adding another nested include.
> +char *ether_ntoa(const struct ether_addr *);
> +struct ether_addr *ether_aton(const char *);
> +int ether_ntohost(char *, const struct ether_addr *);
> +int ether_hostton(const char *, struct ether_addr *);
> +int ether_line(const char *, struct ether_addr *, char *);
> +__END_DECLS
Make that
#ifdef __cplusplus
}
#endif
> +#endif /* !_NETINET_ETHER_H */
No comments in headers, and certainly not this trivial.
> diff --git a/src/network/ethers.c b/src/network/ethers.c
> new file mode 100644
> index 0000000..8014581
> --- /dev/null
> +++ b/src/network/ethers.c
> @@ -0,0 +1,180 @@
> +/* Origin NetBSD: src/lib/libc/net/ethers.c */
> +
> +/*
> + * ethers(3N) a la Sun.
> + *
> + * Written by Roland McGrath <roland@frob.com> 10/14/93.
> + * Public domain.
> + *
> + * port for musl by Abdoulaye Walsimou GAYE <awg@embtoolkit.org>
> 2012/10/15
> + */
> +#define _BSD_SOURCE
> +#include <net/ethernet.h>
> +#include <netinet/ether.h>
> +
> +#include <sys/param.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <paths.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#ifndef _PATH_ETHERS
> +#define _PATH_ETHERS "/etc/ethers"
> +#endif
> +
> +/*
> + * ether_ntoa():
> + * This function converts this structure into an ASCII string of the form
> + * ``xx:xx:xx:xx:xx:xx'', consisting of 6 hexadecimal numbers separated
> + * by colons. It returns a pointer to a static buffer that is reused for
> + * each call.
> + */
> +char *ether_ntoa(const struct ether_addr *e)
> +{
> + static char a[18];
> +
> + assert(e != NULL);
> +
> + (void) snprintf(a, sizeof a, "%02x:%02x:%02x:%02x:%02x:%02x",
> + e->ether_addr_octet[0], e->ether_addr_octet[1],
> + e->ether_addr_octet[2], e->ether_addr_octet[3],
> + e->ether_addr_octet[4], e->ether_addr_octet[5]);
> + return a;
> +}
> +
> +/*
> + * ether_aton():
> + * This function converts an ASCII string of the same form and to a
> structure
> + * containing the 6 octets of the address. It returns a pointer to a
> + * static structure that is reused for each call.
> + */
> +struct ether_addr *ether_aton(const char *s)
> +{
> + static struct ether_addr n;
> + unsigned int i[6];
> +
> + assert(s != NULL);
> +
> + if (sscanf(s, " %x:%x:%x:%x:%x:%x ", &i[0], &i[1],
> + &i[2], &i[3], &i[4], &i[5]) == 6) {
> + n.ether_addr_octet[0] = (unsigned char)i[0];
> + n.ether_addr_octet[1] = (unsigned char)i[1];
> + n.ether_addr_octet[2] = (unsigned char)i[2];
> + n.ether_addr_octet[3] = (unsigned char)i[3];
> + n.ether_addr_octet[4] = (unsigned char)i[4];
> + n.ether_addr_octet[5] = (unsigned char)i[5];
> + return &n;
> + }
> + return NULL;
> +}
> +
> +/*
> + * ether_ntohost():
> + * This function interrogates the data base mapping host names to
> Ethernet
> + * addresses, /etc/ethers.
> + * It looks up the given Ethernet address and writes the associated host
> name
> + * into the character buffer passed.
> + * It returns zero if it finds the requested host name and -1 if not.
> + */
> +int ether_ntohost(char *hostname, const struct ether_addr *e)
> +{
> + FILE *f;
> + char *p;
> + size_t len;
> + struct ether_addr try;
> +
> + assert(hostname != NULL);
> + assert(e != NULL);
> +
> + f = fopen(_PATH_ETHERS, "r");
> + if (f == NULL)
> + return -1;
> + while ((p = fgetln(f, &len)) != NULL) {
> + if (p[len - 1] != '\n')
> + continue; /* skip lines w/o \n */
> + p[--len] = '\0';
> + if (ether_line(p, &try, hostname) == 0 &&
> + memcmp(&try, e, sizeof try) == 0) {
> + (void)fclose(f);
> + return 0;
> + }
> + }
> + (void)fclose(f);
> + errno = ENOENT;
> + return -1;
> +}
> +
> +/*
> + * ether_hostton():
> + * This function interrogates the data base mapping host names to
> Ethernet
> + * addresses, /etc/ethers.
> + * It looks up the given host name and writes the associated Ethernet
> address
> + * into the structure passed.
> + * It returns zero if it finds the requested address and -1 if not.
> + */
> +int ether_hostton(const char *hostname, struct ether_addr *e)
> +{
> + FILE *f;
> + char *p;
> + size_t len;
> + char try[MAXHOSTNAMELEN + 1];
> +
> + assert(hostname != NULL);
> + assert(e != NULL);
> +
> + f = fopen(_PATH_ETHERS, "r");
> + if (f==NULL)
> + return -1;
> +
> + while ((p = fgetln(f, &len)) != NULL) {
> + if (p[len - 1] != '\n')
> + continue; /* skip lines w/o \n */
> + p[--len] = '\0';
> + if (ether_line(p, e, try) == 0 && strcmp(hostname, try) == 0) {
> + (void)fclose(f);
> + return 0;
> + }
> + }
> + (void)fclose(f);
> + errno = ENOENT;
> + return -1;
> +}
> +
> +/*
> + * ether_line():
> + * This function parses a line from the /etc/ethers file and fills in the
> passed
> + * ``struct ether_addr'' and character buffer with the Ethernet address
> and host
> + * name on the line.
> + * It returns zero if the line was successfully parsed and -1 if not.
> + */
> +int ether_line(const char *l, struct ether_addr *e, char *hostname)
> +{
> + unsigned int i[6];
> +
> +#define S2(arg) #arg
> +#define S1(arg) S2(arg)
> + static const char fmt[] = " %x:%x:%x:%x:%x:%x"
> + " %" S1(MAXHOSTNAMELEN) "s\n";
> +#undef S2
> +#undef S1
> +
> + assert(l != NULL);
> + assert(e != NULL);
> + assert(hostname != NULL);
> +
> + if (sscanf(l, fmt,
> + &i[0], &i[1], &i[2], &i[3], &i[4], &i[5], hostname) == 7) {
> + e->ether_addr_octet[0] = (unsigned char)i[0];
> + e->ether_addr_octet[1] = (unsigned char)i[1];
> + e->ether_addr_octet[2] = (unsigned char)i[2];
> + e->ether_addr_octet[3] = (unsigned char)i[3];
> + e->ether_addr_octet[4] = (unsigned char)i[4];
> + e->ether_addr_octet[5] = (unsigned char)i[5];
> + return 0;
> + }
> + errno = EINVAL;
> + return -1;
> +}
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-20 23:37 ` idunham
@ 2012-10-21 0:40 ` Abdoulaye Walsimou GAYE
0 siblings, 0 replies; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-21 0:40 UTC (permalink / raw)
To: musl
On 10/21/2012 01:37 AM, idunham@lavabit.com wrote:
>> +#ifndef _NETINET_ETHER_H
>> +#define _NETINET_ETHER_H
>> +
>> +#include <sys/cdefs.h>
>> +__BEGIN_DECLS
> musl does not do nested includes unless *necessary*, nor do we define
> macros unless it simplifies the code significantly.
> This detail speeds up compilation significantly.
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> is NOT something that obscures the code, and it certainly isn't worth
> adding another nested include.
>
>> +char *ether_ntoa(const struct ether_addr *);
>> +struct ether_addr *ether_aton(const char *);
>> +int ether_ntohost(char *, const struct ether_addr *);
>> +int ether_hostton(const char *, struct ether_addr *);
>> +int ether_line(const char *, struct ether_addr *, char *);
>> +__END_DECLS
> Make that
> #ifdef __cplusplus
> }
> #endif
>
>> +#endif /* !_NETINET_ETHER_H */
> No comments in headers, and certainly not this trivial.
>
>
I see, I will send another patch!
Cheers,
AWG
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-20 20:15 ` [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD Abdoulaye Walsimou Gaye
2012-10-20 23:37 ` idunham
@ 2012-10-21 0:42 ` John Spencer
2012-10-21 0:41 ` Rich Felker
2012-10-21 0:52 ` Abdoulaye Walsimou GAYE
1 sibling, 2 replies; 27+ messages in thread
From: John Spencer @ 2012-10-21 0:42 UTC (permalink / raw)
To: musl
On 10/20/2012 10:15 PM, Abdoulaye Walsimou Gaye wrote:
> +/*
> + * ether_aton():
> + * This function converts an ASCII string of the same form and to a structure
> + * containing the 6 octets of the address. It returns a pointer to a
> + * static structure that is reused for each call.
> + */
> +struct ether_addr *ether_aton(const char *s)
> +{
> + static struct ether_addr n;
> + unsigned int i[6];
> +
> + assert(s != NULL);
> +
> + if (sscanf(s, " %x:%x:%x:%x:%x:%x ",&i[0],&i[1],
> + &i[2],&i[3],&i[4],&i[5]) == 6) {
> + n.ether_addr_octet[0] = (unsigned char)i[0];
> + n.ether_addr_octet[1] = (unsigned char)i[1];
> + n.ether_addr_octet[2] = (unsigned char)i[2];
> + n.ether_addr_octet[3] = (unsigned char)i[3];
> + n.ether_addr_octet[4] = (unsigned char)i[4];
> + n.ether_addr_octet[5] = (unsigned char)i[5];
> + return&n;
> + }
> + return NULL;
> +}
why do you duplicate the code and not use simply the _r functions from
[4/4] with the static buffer ?
btw the usage of sscanf is both bloated and slow.
anyway, i doubt it makes sense to add this crap; i never needed it for
sabotage which is almost feature complete and compiles 250 packages.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-21 0:42 ` John Spencer
@ 2012-10-21 0:41 ` Rich Felker
2012-10-21 0:52 ` Abdoulaye Walsimou GAYE
1 sibling, 0 replies; 27+ messages in thread
From: Rich Felker @ 2012-10-21 0:41 UTC (permalink / raw)
To: musl
On Sun, Oct 21, 2012 at 02:42:52AM +0200, John Spencer wrote:
> On 10/20/2012 10:15 PM, Abdoulaye Walsimou Gaye wrote:
> >+/*
> >+ * ether_aton():
> >+ * This function converts an ASCII string of the same form and to a structure
> >+ * containing the 6 octets of the address. It returns a pointer to a
> >+ * static structure that is reused for each call.
> >+ */
> >+struct ether_addr *ether_aton(const char *s)
> >+{
> >+ static struct ether_addr n;
> >+ unsigned int i[6];
> >+
> >+ assert(s != NULL);
> >+
> >+ if (sscanf(s, " %x:%x:%x:%x:%x:%x ",&i[0],&i[1],
> >+ &i[2],&i[3],&i[4],&i[5]) == 6) {
> >+ n.ether_addr_octet[0] = (unsigned char)i[0];
> >+ n.ether_addr_octet[1] = (unsigned char)i[1];
> >+ n.ether_addr_octet[2] = (unsigned char)i[2];
> >+ n.ether_addr_octet[3] = (unsigned char)i[3];
> >+ n.ether_addr_octet[4] = (unsigned char)i[4];
> >+ n.ether_addr_octet[5] = (unsigned char)i[5];
> >+ return&n;
> >+ }
> >+ return NULL;
> >+}
>
> why do you duplicate the code and not use simply the _r functions
> from [4/4] with the static buffer ?
The 4/4 patch removes this code and replaces it with a call to the _r
version.
> btw the usage of sscanf is both bloated and slow.
I disagree. If these functions are rarely used, delegating the work to
sscanf avoids adding bloated parsing code. Yes it means applications
that use these functions will pull in the scanf framework, but...
> anyway, i doubt it makes sense to add this crap; i never needed it
> for sabotage which is almost feature complete and compiles 250
> packages.
...as you just said, such applications are very rare.
Anyway, I don't think it's constructive to call it "crap". Presumably
there are some programs, most likely network configuration tools,
packet sniffers, etc. that use this code to format ethernet addresses
for printing, which is a reasonable need. I do question whether we
need to support /etc/ethers (I've never heard of anybody using it),
but I don't think immediately classifying attempts at contribution as
"crap" is conducive to building a good community.
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-21 0:42 ` John Spencer
2012-10-21 0:41 ` Rich Felker
@ 2012-10-21 0:52 ` Abdoulaye Walsimou GAYE
2012-10-21 0:48 ` Rich Felker
2012-10-21 0:57 ` John Spencer
1 sibling, 2 replies; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-21 0:52 UTC (permalink / raw)
To: musl
On 10/21/2012 02:42 AM, John Spencer wrote:
> On 10/20/2012 10:15 PM, Abdoulaye Walsimou Gaye wrote:
>> +/*
>> + * ether_aton():
>> + * This function converts an ASCII string of the same form and to a structure
>> + * containing the 6 octets of the address. It returns a pointer to a
>> + * static structure that is reused for each call.
>> + */
>> +struct ether_addr *ether_aton(const char *s)
>> +{
>> + static struct ether_addr n;
>> + unsigned int i[6];
>> +
>> + assert(s != NULL);
>> +
>> + if (sscanf(s, " %x:%x:%x:%x:%x:%x ",&i[0],&i[1],
>> + &i[2],&i[3],&i[4],&i[5]) == 6) {
>> + n.ether_addr_octet[0] = (unsigned char)i[0];
>> + n.ether_addr_octet[1] = (unsigned char)i[1];
>> + n.ether_addr_octet[2] = (unsigned char)i[2];
>> + n.ether_addr_octet[3] = (unsigned char)i[3];
>> + n.ether_addr_octet[4] = (unsigned char)i[4];
>> + n.ether_addr_octet[5] = (unsigned char)i[5];
>> + return&n;
>> + }
>> + return NULL;
>> +}
>
> why do you duplicate the code and not use simply the _r functions from [4/4] with the static buffer ?
> btw the usage of sscanf is both bloated and slow.
This was for record of the original code from NetBSD, before patching it in [4/4]
>
> anyway, i doubt it makes sense to add this crap; i never needed it for sabotage which is almost feature complete and compiles 250 packages.
I can not argue with this kind of comments.
Cheers,
AWG
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-21 0:52 ` Abdoulaye Walsimou GAYE
@ 2012-10-21 0:48 ` Rich Felker
2012-10-21 0:57 ` John Spencer
1 sibling, 0 replies; 27+ messages in thread
From: Rich Felker @ 2012-10-21 0:48 UTC (permalink / raw)
To: musl
On Sun, Oct 21, 2012 at 02:52:21AM +0200, Abdoulaye Walsimou GAYE wrote:
> >why do you duplicate the code and not use simply the _r functions from [4/4] with the static buffer ?
> >btw the usage of sscanf is both bloated and slow.
>
> This was for record of the original code from NetBSD, before
> patching it in [4/4]
Understood.
> >anyway, i doubt it makes sense to add this crap; i never needed it
> >for sabotage which is almost feature complete and compiles 250
> >packages.
>
> I can not argue with this kind of comments.
Apologies; some members of our community are a bit abrasive at times,
and I think your initial insistence on the cdefs.h thing (which was
discussed to death in the past) got a few ppl irritated.
I have no objection to the basic functionality as long as it's done in
a way that's clean and unobtrusive. I think it would be nice to know
what apps/libs need it though, as a justification. Additions like this
(i.e. functions not required by the standards and not widely used)
should be documented as added "because it's needed by such-and-such"
rather than "just because we can".
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-21 0:52 ` Abdoulaye Walsimou GAYE
2012-10-21 0:48 ` Rich Felker
@ 2012-10-21 0:57 ` John Spencer
2012-10-21 0:53 ` Rich Felker
1 sibling, 1 reply; 27+ messages in thread
From: John Spencer @ 2012-10-21 0:57 UTC (permalink / raw)
To: musl
On 10/21/2012 02:52 AM, Abdoulaye Walsimou GAYE wrote:
> On 10/21/2012 02:42 AM, John Spencer wrote:
>> On 10/20/2012 10:15 PM, Abdoulaye Walsimou Gaye wrote:
>>> +/*
>>> + * ether_aton():
>>> + * This function converts an ASCII string of the same form and to a
>>> structure
>>> + * containing the 6 octets of the address. It returns a pointer to a
>>> + * static structure that is reused for each call.
>>> + */
>>> +struct ether_addr *ether_aton(const char *s)
>>> +{
>>> + static struct ether_addr n;
>>> + unsigned int i[6];
>>> +
>>> + assert(s != NULL);
>>> +
>>> + if (sscanf(s, " %x:%x:%x:%x:%x:%x ",&i[0],&i[1],
>>> + &i[2],&i[3],&i[4],&i[5]) == 6) {
>>> + n.ether_addr_octet[0] = (unsigned char)i[0];
>>> + n.ether_addr_octet[1] = (unsigned char)i[1];
>>> + n.ether_addr_octet[2] = (unsigned char)i[2];
>>> + n.ether_addr_octet[3] = (unsigned char)i[3];
>>> + n.ether_addr_octet[4] = (unsigned char)i[4];
>>> + n.ether_addr_octet[5] = (unsigned char)i[5];
>>> + return&n;
>>> + }
>>> + return NULL;
>>> +}
>>
>> why do you duplicate the code and not use simply the _r functions
>> from [4/4] with the static buffer ?
>> btw the usage of sscanf is both bloated and slow.
>
> This was for record of the original code from NetBSD, before patching
> it in [4/4]
ah... so you sent patches that undo your earlier patches.
this doesnt make reviewing them simpler...
why didn't you simply merge them before sending ?
>
>>
>> anyway, i doubt it makes sense to add this crap; i never needed it
>> for sabotage which is almost feature complete and compiles 250 packages.
>
> I can not argue with this kind of comments.
with crap, i was refering to these functions, not your work.
the point that i haven't needed these functions besides having
wpa_supplicant, wireless-tools, iptables and nmap in the repos still stands.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD
2012-10-21 0:57 ` John Spencer
@ 2012-10-21 0:53 ` Rich Felker
0 siblings, 0 replies; 27+ messages in thread
From: Rich Felker @ 2012-10-21 0:53 UTC (permalink / raw)
To: musl
On Sun, Oct 21, 2012 at 02:57:44AM +0200, John Spencer wrote:
> >This was for record of the original code from NetBSD, before
> >patching it in [4/4]
>
> ah... so you sent patches that undo your earlier patches.
> this doesnt make reviewing them simpler...
> why didn't you simply merge them before sending ?
I agree, reviewing would have been easier with them merged, but I can
understand the interest in documenting the original source of the
code.
> >>anyway, i doubt it makes sense to add this crap; i never needed
> >>it for sabotage which is almost feature complete and compiles
> >>250 packages.
> >
> >I can not argue with this kind of comments.
>
> with crap, i was refering to these functions, not your work.
> the point that i haven't needed these functions besides having
> wpa_supplicant, wireless-tools, iptables and nmap in the repos still
> stands.
Note that gnulib has replacements for these functions, so if there are
packages using gnulib which use them, they might just be pulling in
the gnulib versions. Just a guess. Again, I would like to know some
real-world examples of applications using them.
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()
2012-10-20 20:15 [PATCH 0/4] Import <netinet/ether.h> features from NetBSD Abdoulaye Walsimou Gaye
` (2 preceding siblings ...)
2012-10-20 20:15 ` [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD Abdoulaye Walsimou Gaye
@ 2012-10-20 20:15 ` Abdoulaye Walsimou Gaye
2012-10-20 23:53 ` Rich Felker
3 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou Gaye @ 2012-10-20 20:15 UTC (permalink / raw)
To: musl; +Cc: Abdoulaye Walsimou Gaye
Signed-off-by: Abdoulaye Walsimou Gaye <awg@embtoolkit.org>
---
include/netinet/ether.h | 2 ++
src/network/ethers.c | 59 +++++++++++++++++++++++++++++++++++------------
2 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/include/netinet/ether.h b/include/netinet/ether.h
index 44c614e..55be0d8 100644
--- a/include/netinet/ether.h
+++ b/include/netinet/ether.h
@@ -5,7 +5,9 @@
__BEGIN_DECLS
char *ether_ntoa(const struct ether_addr *);
+char *ether_ntoa_r(const struct ether_addr *, char *);
struct ether_addr *ether_aton(const char *);
+struct ether_addr *ether_aton_r(const char *, struct ether_addr *);
int ether_ntohost(char *, const struct ether_addr *);
int ether_hostton(const char *, struct ether_addr *);
int ether_line(const char *, struct ether_addr *, char *);
diff --git a/src/network/ethers.c b/src/network/ethers.c
index 8014581..930712e 100644
--- a/src/network/ethers.c
+++ b/src/network/ethers.c
@@ -36,13 +36,25 @@ char *ether_ntoa(const struct ether_addr *e)
{
static char a[18];
+ return ether_ntoa_r(e, a);
+}
+
+/*
+ * ether_ntoa_r():
+ * Thread safe version of ether_ntoa() (does not make use of static buffer).
+ */
+char *ether_ntoa_r(const struct ether_addr *e, char *a)
+{
+ int sz;
assert(e != NULL);
+ assert(a != NULL);
+
+ sz = sprintf(a, "%02x:%02x:%02x:%02x:%02x:%02x",
+ e->ether_addr_octet[0], e->ether_addr_octet[1],
+ e->ether_addr_octet[2], e->ether_addr_octet[3],
+ e->ether_addr_octet[4], e->ether_addr_octet[5]);
- (void) snprintf(a, sizeof a, "%02x:%02x:%02x:%02x:%02x:%02x",
- e->ether_addr_octet[0], e->ether_addr_octet[1],
- e->ether_addr_octet[2], e->ether_addr_octet[3],
- e->ether_addr_octet[4], e->ether_addr_octet[5]);
- return a;
+ return sz < 17 ? NULL : a;
}
/*
@@ -54,20 +66,37 @@ char *ether_ntoa(const struct ether_addr *e)
struct ether_addr *ether_aton(const char *s)
{
static struct ether_addr n;
- unsigned int i[6];
assert(s != NULL);
- if (sscanf(s, " %x:%x:%x:%x:%x:%x ", &i[0], &i[1],
- &i[2], &i[3], &i[4], &i[5]) == 6) {
- n.ether_addr_octet[0] = (unsigned char)i[0];
- n.ether_addr_octet[1] = (unsigned char)i[1];
- n.ether_addr_octet[2] = (unsigned char)i[2];
- n.ether_addr_octet[3] = (unsigned char)i[3];
- n.ether_addr_octet[4] = (unsigned char)i[4];
- n.ether_addr_octet[5] = (unsigned char)i[5];
- return &n;
+ return ether_aton_r(s, &n);
+}
+
+/*
+ * ether_aton_r():
+ * Thread safe version of ether_aton(), (does not make use of static structure).
+ */
+struct ether_addr *ether_aton_r(const char *s, struct ether_addr *n)
+{
+ unsigned int i[6];
+ int sz;
+
+ assert(s != NULL);
+ assert(n != NULL);
+
+ sz = sscanf(s, " %x:%x:%x:%x:%x:%x ",
+ &i[0], &i[1], &i[2], &i[3], &i[4], &i[5]);
+
+ if (sz == 6) {
+ n->ether_addr_octet[0] = (unsigned char)i[0];
+ n->ether_addr_octet[1] = (unsigned char)i[1];
+ n->ether_addr_octet[2] = (unsigned char)i[2];
+ n->ether_addr_octet[3] = (unsigned char)i[3];
+ n->ether_addr_octet[4] = (unsigned char)i[4];
+ n->ether_addr_octet[5] = (unsigned char)i[5];
+ return n;
}
+
return NULL;
}
--
1.7.9.5
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()
2012-10-20 20:15 ` [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r() Abdoulaye Walsimou Gaye
@ 2012-10-20 23:53 ` Rich Felker
2012-10-21 0:43 ` Abdoulaye Walsimou GAYE
0 siblings, 1 reply; 27+ messages in thread
From: Rich Felker @ 2012-10-20 23:53 UTC (permalink / raw)
To: musl
On Sat, Oct 20, 2012 at 10:15:45PM +0200, Abdoulaye Walsimou Gaye wrote:
> +struct ether_addr *ether_aton_r(const char *s, struct ether_addr *n)
> +{
> + unsigned int i[6];
> + int sz;
> +
> + assert(s != NULL);
> + assert(n != NULL);
> +
> + sz = sscanf(s, " %x:%x:%x:%x:%x:%x ",
> + &i[0], &i[1], &i[2], &i[3], &i[4], &i[5]);
> +
> + if (sz == 6) {
> + n->ether_addr_octet[0] = (unsigned char)i[0];
> + n->ether_addr_octet[1] = (unsigned char)i[1];
> + n->ether_addr_octet[2] = (unsigned char)i[2];
> + n->ether_addr_octet[3] = (unsigned char)i[3];
> + n->ether_addr_octet[4] = (unsigned char)i[4];
> + n->ether_addr_octet[5] = (unsigned char)i[5];
> + return n;
I think this code could be greatly simplified using %hhx as the format
instead of %x, but either way it's lacking any error checking and
accepts lots of inputs that probably should not be accepted. Is this
an issue?
Rich
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()
2012-10-20 23:53 ` Rich Felker
@ 2012-10-21 0:43 ` Abdoulaye Walsimou GAYE
2012-10-21 0:46 ` John Spencer
0 siblings, 1 reply; 27+ messages in thread
From: Abdoulaye Walsimou GAYE @ 2012-10-21 0:43 UTC (permalink / raw)
To: musl
On 10/21/2012 01:53 AM, Rich Felker wrote:
> On Sat, Oct 20, 2012 at 10:15:45PM +0200, Abdoulaye Walsimou Gaye wrote:
>> +struct ether_addr *ether_aton_r(const char *s, struct ether_addr *n)
>> +{
>> + unsigned int i[6];
>> + int sz;
>> +
>> + assert(s != NULL);
>> + assert(n != NULL);
>> +
>> + sz = sscanf(s, " %x:%x:%x:%x:%x:%x ",
>> + &i[0], &i[1], &i[2], &i[3], &i[4], &i[5]);
>> +
>> + if (sz == 6) {
>> + n->ether_addr_octet[0] = (unsigned char)i[0];
>> + n->ether_addr_octet[1] = (unsigned char)i[1];
>> + n->ether_addr_octet[2] = (unsigned char)i[2];
>> + n->ether_addr_octet[3] = (unsigned char)i[3];
>> + n->ether_addr_octet[4] = (unsigned char)i[4];
>> + n->ether_addr_octet[5] = (unsigned char)i[5];
>> + return n;
> I think this code could be greatly simplified using %hhx as the format
> instead of %x, but either way it's lacking any error checking and
> accepts lots of inputs that probably should not be accepted. Is this
> an issue?
>
> Rich
>
>
As I said, the code is almost copied "AS IS" from NetBSD to have <netinet/ether.h> features,
feel free to amend it.
cheers,
AWG
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()
2012-10-21 0:43 ` Abdoulaye Walsimou GAYE
@ 2012-10-21 0:46 ` John Spencer
0 siblings, 0 replies; 27+ messages in thread
From: John Spencer @ 2012-10-21 0:46 UTC (permalink / raw)
To: musl
On 10/21/2012 02:43 AM, Abdoulaye Walsimou GAYE wrote:
> On 10/21/2012 01:53 AM, Rich Felker wrote:
>> On Sat, Oct 20, 2012 at 10:15:45PM +0200, Abdoulaye Walsimou Gaye wrote:
>>> +struct ether_addr *ether_aton_r(const char *s, struct ether_addr *n)
>>> +{
>>> + unsigned int i[6];
>>> + int sz;
>>> +
>>> + assert(s != NULL);
>>> + assert(n != NULL);
>>> +
>>> + sz = sscanf(s, " %x:%x:%x:%x:%x:%x ",
>>> + &i[0], &i[1], &i[2], &i[3], &i[4], &i[5]);
>>> +
>>> + if (sz == 6) {
>>> + n->ether_addr_octet[0] = (unsigned char)i[0];
>>> + n->ether_addr_octet[1] = (unsigned char)i[1];
>>> + n->ether_addr_octet[2] = (unsigned char)i[2];
>>> + n->ether_addr_octet[3] = (unsigned char)i[3];
>>> + n->ether_addr_octet[4] = (unsigned char)i[4];
>>> + n->ether_addr_octet[5] = (unsigned char)i[5];
>>> + return n;
>> I think this code could be greatly simplified using %hhx as the format
>> instead of %x, but either way it's lacking any error checking and
>> accepts lots of inputs that probably should not be accepted. Is this
>> an issue?
>>
>> Rich
>>
>>
>
> As I said, the code is almost copied "AS IS" from NetBSD to have
> <netinet/ether.h> features,
> feel free to amend it.
s/amend/reject/
^ permalink raw reply [flat|nested] 27+ messages in thread