Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 5 May 2013 09:02:04 -0500
From: Strake <strake888@...il.com>
To: musl@...ts.openwall.com
Subject: Re: [patch] add ether_(aton, ntoa)

On 19/04/2013, Rich Felker <dalias@...ifal.cx> wrote:
> On Sun, Apr 14, 2013 at 11:10:55PM -0500, Strake wrote:
>> +struct ether_addr *ether_aton_r (const char *x, struct ether_addr *p_a)
>> {
>> +	for (int ii = 0; ii < 6; ii++) {
>> +		unsigned long int n;
>> +		if (ii != 0) {
>> +			if (x[0] != ':') return 0; /* bad format */
>> +			else x++;
>> +		}
>> +		n = strtoul (x, &x, 16);
>
> &x has the wrong type; strtoul requires char **, not const char **.
> A separate temp var is required for the output, as in.
>
> 	char *y;
> 	n = strtoul (x, &y, 16);
> 	x = y;
>
> or similar. As far as I know the naive cast one could make to "fix"
> the type mismatch is not valid; it would be an aliasing violation.

What bad could happen?

>> +		if (n > 0xFF) return 0; /* bad byte */
>> +		(*p_a).ether_addr_octet[ii] = n;
>
> Is there a reason you're using (*p_a). rather than p_a-> ?

No sane one.

> Also, I'm not sure if this is worth changing or not, but usually we
> avoid writing to output pointers except on success. For standard
> interfaces this is a conformance issue, but for these it probably
> doesn't matter.

Well, we may as well do it properly.

>From 97d977299521a0ae7cc23deed16dabfd22363248 Mon Sep 17 00:00:00 2001
From: Strake <strake888@...il.com>
Date: Sun, 14 Apr 2013 12:09:30 -0500
Subject: [PATCH] add ether_(aton, ntoa)

---
 include/netinet/ether.h  | 14 ++++++++++++++
 src/network/ether_aton.c | 26 ++++++++++++++++++++++++++
 src/network/ether_ntoa.c | 17 +++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 include/netinet/ether.h
 create mode 100644 src/network/ether_aton.c
 create mode 100644 src/network/ether_ntoa.c

diff --git a/include/netinet/ether.h b/include/netinet/ether.h
new file mode 100644
index 0000000..c5179d5
--- /dev/null
+++ b/include/netinet/ether.h
@@ -0,0 +1,14 @@
+#ifndef _NETINET_ETHER_H
+#define _NETINET_ETHER_H
+
+#include <netinet/if_ether.h>
+
+char *ether_ntoa (const struct ether_addr *);
+
+struct ether_addr *ether_aton (const char *);
+
+char *ether_ntoa_r (const struct ether_addr *, char *);
+
+struct ether_addr *ether_aton_r (const char *, struct ether_addr *);
+
+#endif
diff --git a/src/network/ether_aton.c b/src/network/ether_aton.c
new file mode 100644
index 0000000..2690cc5
--- /dev/null
+++ b/src/network/ether_aton.c
@@ -0,0 +1,26 @@
+#include <stdlib.h>
+#include <string.h>
+#include <netinet/ether.h>
+
+static struct ether_addr a;
+
+struct ether_addr *ether_aton_r (const char *x, struct ether_addr *p_a) {
+	struct ether_addr a;
+	for (int ii = 0; ii < 6; ii++) {
+		unsigned long int n;
+		if (ii != 0) {
+			if (x[0] != ':') return 0; /* bad format */
+			else x++;
+		}
+		n = strtoul (x, &x, 16);
+		if (n > 0xFF) return 0; /* bad byte */
+		a.ether_addr_octet[ii] = n;
+	}
+	if (x[0] != 0) return 0; /* bad format */
+	memmove (p_a, &a, sizeof (struct ether_addr));
+	return p_a;
+}
+
+struct ether_addr *ether_aton (const char *x) {
+	return ether_aton_r (x, &a);
+}
diff --git a/src/network/ether_ntoa.c b/src/network/ether_ntoa.c
new file mode 100644
index 0000000..bcc773a
--- /dev/null
+++ b/src/network/ether_ntoa.c
@@ -0,0 +1,17 @@
+#include <stdio.h>
+#include <netinet/ether.h>
+
+static char x[18];
+
+char *ether_ntoa_r (const struct ether_addr *p_a, char *x) {
+	char *y;
+	y = x;
+	for (int ii = 0; ii < 6; ii++) {
+		x += sprintf (x, ii == 0 ? "%.2X" : ":%.2X", p_a -> ether_addr_octet[ii]);
+	}
+	return y;
+}
+
+char *ether_ntoa (const struct ether_addr *p_a) {
+	return ether_ntoa_r (p_a, x);
+}
-- 
1.7.11.1

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.