[PATCH 0/4] Import <netinet/ether.h> features from NetBSD

[PATCH 0/4] Import <netinet/ether.h> features from NetBSD

[Abdoulaye Walsimou Gaye]

This set of patches mainly adds <netinet/ether.h> features imported from NetBSD.
These functions are almost imported "AS IS" and are, for the moment, only
compile tested.

Abdoulaye Walsimou Gaye (4):
  Build system: give ability to install lib/crt*.o files separately
  Add basic sys/cdefs.h found on most unix
  Import BSD fonctions defined in <netinet/ether.h> from NetBSD
  <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()

 Makefile                           |    1 +
 include/netinet/ether.h            |   16 +++
 include/sys/cdefs.h                |   22 ++++
 src/network/ethers.c               |  209 ++++++++++++++++++++++++++++++++++++
 5 files changed, 261 insertions(+)
 create mode 100644 include/netinet/ether.h
 create mode 100644 include/sys/cdefs.h
 create mode 100644 src/network/ethers.c

-- 
1.7.9.5

[PATCH 1/4] Build system: give ability to install lib/crt*.o files separately

[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

[PATCH 2/4] Add basic sys/cdefs.h found on most unix

[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

[PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[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

[PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()

[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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Isaac Dunham]

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>

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[idunham]

> +#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;
> +}

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Abdoulaye Walsimou GAYE]

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,

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Rich Felker]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Rich Felker]

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

Re: [PATCH 1/4] Build system: give ability to install lib/crt*.o files separately

[Rich Felker]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Isaac Dunham]

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>

Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()

[Rich Felker]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Rich Felker]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Abdoulaye Walsimou GAYE]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[Abdoulaye Walsimou GAYE]

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

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[John Spencer]

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.

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[Abdoulaye Walsimou GAYE]

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

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[Rich Felker]

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

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[John Spencer]

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.

Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()

[Abdoulaye Walsimou GAYE]

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

Re: [PATCH 4/4] <netinet/ether.h>: Add GNU extensions ether_ntoa_r() and ether_aton_r()

[John Spencer]

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/

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[Rich Felker]

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

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[Abdoulaye Walsimou GAYE]

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

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[Rich Felker]

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

Re: [PATCH 3/4] Import BSD functions defined in <netinet/ether.h> from NetBSD

[John Spencer]

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.

Re: [PATCH 2/4] Add basic sys/cdefs.h found on most unix

[John Spencer]

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)