arptables 0.0.3-4 build fixes

arptables 0.0.3-4 build fixes

[Jan Engelhardt]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 391 bytes --]

Hi,


from my side two build patches for arptables.

(1) Do not call install with -o or -g, because it makes unprivileged 
installs (used by build chroots) fail. Autotools does not use -o or -g 
either, so the usage is consistent and does what was inteded when root
is running `make install`.

(2) Fixing all the compiler warnings that turned up about printf 
formatting and strict-aliasing.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-patch; name=arptables-install.diff, Size: 1700 bytes --]

---
 Makefile |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: arptables-v0.0.3-4/Makefile
===================================================================
--- arptables-v0.0.3-4.orig/Makefile
+++ arptables-v0.0.3-4/Makefile
@@ -35,22 +35,22 @@ arptables: arptables-standalone.o arptab
 
 $(DESTDIR)$(MANDIR)/man8/arptables.8: arptables.8
 	mkdir -p $(@D)
-	install -m 0644 -o root -g root $< $@
+	install -m 0644 $< $@
 
 $(DESTDIR)$(BINDIR)/arptables: arptables
 	mkdir -p $(DESTDIR)$(BINDIR)
-	install -m 0755 -o root -g root $< $@
+	install -m 0755 $< $@
 
 tmp1:=$(shell printf $(BINDIR) | sed 's/\//\\\//g')
 tmp2:=$(shell printf $(SYSCONFIGDIR) | sed 's/\//\\\//g')
 .PHONY: scripts
 scripts: arptables-save arptables-restore arptables.sysv
 	cat arptables-save | sed 's/__EXEC_PATH__/$(tmp1)/g' > arptables-save_
-	install -m 0755 -o root -g root arptables-save_ $(DESTDIR)$(BINDIR)/arptables-save
+	install -m 0755 arptables-save_ $(DESTDIR)$(BINDIR)/arptables-save
 	cat arptables-restore | sed 's/__EXEC_PATH__/$(tmp1)/g' > arptables-restore_
-	install -m 0755 -o root -g root arptables-restore_ $(DESTDIR)$(BINDIR)/arptables-restore
+	install -m 0755 arptables-restore_ $(DESTDIR)$(BINDIR)/arptables-restore
 	cat arptables.sysv | sed 's/__EXEC_PATH__/$(tmp1)/g' | sed 's/__SYSCONFIG__/$(tmp2)/g' > arptables.sysv_
-	if test -d $(DESTDIR)$(INITDIR); then install -m 0755 -o root -g root arptables.sysv_ $(DESTDIR)$(INITDIR)/arptables; fi
+	if test -d $(DESTDIR)$(INITDIR); then install -m 0755 arptables.sysv_ $(DESTDIR)$(INITDIR)/arptables; fi
 	rm -f arptables-save_ arptables-restore_ arptables.sysv_
 
 .PHONY: install

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: TEXT/x-patch; name=arptables-warnings.diff, Size: 5718 bytes --]

---
 arptables.c              |   27 +++++++++++++++------------
 libarptc/libarptc.c      |    6 ++++--
 libarptc/libarptc_incl.c |    6 ++++--
 3 files changed, 23 insertions(+), 16 deletions(-)

Index: arptables-v0.0.3-4/arptables.c
===================================================================
--- arptables-v0.0.3-4.orig/arptables.c
+++ arptables-v0.0.3-4/arptables.c
@@ -874,7 +874,7 @@ parse_target(const char *targetname)
 
 	if (strlen(targetname)+1 > sizeof(arpt_chainlabel))
 		exit_error(PARAMETER_PROBLEM,
-			   "Invalid target name `%s' (%i chars max)",
+			   "Invalid target name `%s' (%zu chars max)",
 			   targetname, sizeof(arpt_chainlabel)-1);
 
 	for (ptr = targetname; *ptr; ptr++)
@@ -1062,7 +1062,7 @@ register_match(struct arptables_match *m
 	}
 
 	if (me->size != ARPT_ALIGN(me->size)) {
-		fprintf(stderr, "%s: match `%s' has invalid size %u.\n",
+		fprintf(stderr, "%s: match `%s' has invalid size %zu.\n",
 			program_name, me->name, me->size);
 		exit(1);
 	}
@@ -1092,7 +1092,7 @@ register_target(struct arptables_target
 	}
 
 	if (me->size != ARPT_ALIGN(me->size)) {
-		fprintf(stderr, "%s: target `%s' has invalid size %u.\n",
+		fprintf(stderr, "%s: target `%s' has invalid size %zu.\n",
 			program_name, me->name, me->size);
 		exit(1);
 	}
@@ -1116,17 +1116,17 @@ print_num(u_int64_t number, unsigned int
 					number = (number + 500) / 1000;
 					if (number > 9999) {
 						number = (number + 500) / 1000;
-						printf(FMT("%4lluT ","%lluT "), number);
+						printf(FMT("%4lluT ","%lluT "), (unsigned long long)number);
 					}
-					else printf(FMT("%4lluG ","%lluG "), number);
+					else printf(FMT("%4lluG ","%lluG "), (unsigned long long)number);
 				}
-				else printf(FMT("%4lluM ","%lluM "), number);
+				else printf(FMT("%4lluM ","%lluM "), (unsigned long long)number);
 			} else
-				printf(FMT("%4lluK ","%lluK "), number);
+				printf(FMT("%4lluK ","%lluK "), (unsigned long long)number);
 		} else
-			printf(FMT("%5llu ","%llu "), number);
+			printf(FMT("%5llu ","%llu "), (unsigned long long)number);
 	} else
-		printf(FMT("%8llu ","%llu "), number);
+		printf(FMT("%8llu ","%llu "), (unsigned long long)number);
 }
 
 
@@ -1370,7 +1370,7 @@ after_devdst:
 			/* Print the target information. */
 			target->print(&fw->arp, t, format & FMT_NUMERIC);
 	} else if (t->u.target_size != sizeof(*t))
-		printf("[%u bytes of unknown target data] ",
+		printf("[%zu bytes of unknown target data] ",
 		       t->u.target_size - sizeof(*t));
 
 	if (!(format & FMT_NOCOUNTS)) {
@@ -1777,6 +1777,7 @@ int do_command(int argc, char *argv[], c
 	const char *jumpto = "";
 	char *protocol = NULL;
 	const char *modprobe = NULL;
+	unsigned long long bpcnt_num;
 
 	/* first figure out if this is a 2.6 or a 2.4 kernel */
 	*handle = arptc_init(*table);
@@ -2159,15 +2160,17 @@ int do_command(int argc, char *argv[], c
 					"-%c requires packet and byte counter",
 					opt2char(OPT_COUNTERS));
 
-			if (sscanf(pcnt, "%llu", &fw.counters.pcnt) != 1)
+			if (sscanf(pcnt, "%llu", &bpcnt_num) != 1)
 				exit_error(PARAMETER_PROBLEM,
 					"-%c packet counter not numeric",
 					opt2char(OPT_COUNTERS));
+			fw.counters.pcnt = bpcnt_num;
 
-			if (sscanf(bcnt, "%llu", &fw.counters.bcnt) != 1)
+			if (sscanf(bcnt, "%llu", &bpcnt_num) != 1)
 				exit_error(PARAMETER_PROBLEM,
 					"-%c byte counter not numeric",
 					opt2char(OPT_COUNTERS));
+			fw.counters.bcnt = bpcnt_num;
 			
 			break;
 
Index: arptables-v0.0.3-4/libarptc/libarptc.c
===================================================================
--- arptables-v0.0.3-4.orig/libarptc/libarptc.c
+++ arptables-v0.0.3-4/libarptc/libarptc.c
@@ -133,7 +133,8 @@ dump_entry(STRUCT_ENTRY *e, const TC_HAN
 	printf("Flags: %02X\n", e->arp.flags);
 	printf("Invflags: %02X\n", e->arp.invflags);
 	printf("Counters: %llu packets, %llu bytes\n",
-	       e->counters.pcnt, e->counters.bcnt);
+		(unsigned long long)e->counters.pcnt, 
+		(unsigned long long)e->counters.bcnt);
 /*
 	printf("Cache: %08X ", e->nfcache);
 	if (e->nfcache & NFC_ALTERED) printf("ALTERED ");
@@ -159,7 +160,8 @@ dump_entry(STRUCT_ENTRY *e, const TC_HAN
 	t = GET_TARGET(e);
 	printf("Target name: `%s' [%u]\n", t->u.user.name, t->u.target_size);
 	if (strcmp(t->u.user.name, STANDARD_TARGET) == 0) {
-		int pos = *(int *)t->data;
+		const unsigned char *data = t->data;
+		int pos = *(const int *)data;
 		if (pos < 0)
 			printf("verdict=%s\n",
 			       pos == -NF_ACCEPT-1 ? "NF_ACCEPT"
Index: arptables-v0.0.3-4/libarptc/libarptc_incl.c
===================================================================
--- arptables-v0.0.3-4.orig/libarptc/libarptc_incl.c
+++ arptables-v0.0.3-4/libarptc/libarptc_incl.c
@@ -121,7 +121,7 @@ entry2index(const TC_HANDLE_T h, const S
 
 	if (ENTRY_ITERATE(h->entries.entrytable, h->entries.size,
 			  get_number, seek, &pos) == 0) {
-		fprintf(stderr, "ERROR: offset %i not an entry!\n",
+		fprintf(stderr, "ERROR: offset %zu not an entry!\n",
 			(char *)seek - (char *)h->entries.entrytable);
 		abort();
 	}
@@ -583,6 +583,7 @@ target_name(TC_HANDLE_T handle, const ST
 	int spos;
 	unsigned int labelidx;
 	STRUCT_ENTRY *jumpto;
+	const unsigned char *data;
 
 	/* To avoid const warnings */
 	STRUCT_ENTRY *e = (STRUCT_ENTRY *)ce;
@@ -591,7 +592,8 @@ target_name(TC_HANDLE_T handle, const ST
 		return GET_TARGET(e)->u.user.name;
 
 	/* Standard target: evaluate */
-	spos = *(int *)GET_TARGET(e)->data;
+	data = GET_TARGET(e)->data;
+	spos = *(const int *)data;
 	if (spos < 0) {
 		if (spos == RETURN)
 			return LABEL_RETURN;

Re: arptables 0.0.3-4 build fixes

[Bart De Schuymer]

Jan Engelhardt wrote:
> Hi,
> 
> 
> from my side two build patches for arptables.
> 
> (1) Do not call install with -o or -g, because it makes unprivileged 
> installs (used by build chroots) fail. Autotools does not use -o or -g 
> either, so the usage is consistent and does what was inteded when root
> is running `make install`.
> 
> (2) Fixing all the compiler warnings that turned up about printf 
> formatting and strict-aliasing.
> 

Hi,

Someone once insisted using this -o -g usage long ago and no
distributors have complained about it yet, so I'm going to leave it in
for now. Plenty of other projects do it this way too.

The warings that occur when using llu with 64-bit numbers should be
resolved with the PRIu64 macro. Could you fix that and resubmit, please?
Thanks.

cheers,
Bart


-- 
Bart De Schuymer
www.artinalgorithms.be

Re: arptables 0.0.3-4 build fixes

[Jan Engelhardt]

On Tuesday 2010-03-16 14:16, Bart De Schuymer wrote:
>> 
>> from my side two build patches for arptables.
>> 
>> (1) Do not call install with -o or -g, because it makes unprivileged 
>> installs (used by build chroots) fail. Autotools does not use -o or -g 
>> either, so the usage is consistent and does what was inteded when root
>> is running `make install`.
>> 
>> (2) Fixing all the compiler warnings that turned up about printf 
>> formatting and strict-aliasing.
>
>Someone once insisted using this -o -g usage long ago and no
>distributors have complained about it yet,

Fedora ships arptables_jf, which does not use -o,-g, thus does not have
a problem to report.

OpenSUSE shipped arptables_jf. I moved it back to arptables
because the jf fork is in an uncertain state.

Debian either builds with root or has patched install(1).

>so I'm going to leave it in
>for now. Plenty of other projects do it this way too.

Well it is wrong, and I can list many more projects (namely:
everything using automake's built-in installation) that do it the
right way instead.

If you are still not convinced: running automake's `make install`
as root will (which you will have to do to even be eligible to
install it to system locations), surprise surprise, install the
files as root. So it's doing the right thing in both cases.

>The warings that occur when using llu with 64-bit numbers should be
>resolved with the PRIu64 macro. Could you fix that and resubmit, please?
>Thanks.

Hmm, iptables is not using PRI either so it seemed like the preferred
approach.

Re: arptables 0.0.3-4 build fixes

[Bart De Schuymer]

Jan Engelhardt wrote:
> On Tuesday 2010-03-16 14:16, Bart De Schuymer wrote:
>>> from my side two build patches for arptables.
>>>
>>> (1) Do not call install with -o or -g, because it makes unprivileged 
>>> installs (used by build chroots) fail. Autotools does not use -o or -g 
>>> either, so the usage is consistent and does what was inteded when root
>>> is running `make install`.
>>>
>>> (2) Fixing all the compiler warnings that turned up about printf 
>>> formatting and strict-aliasing.
>> Someone once insisted using this -o -g usage long ago and no
>> distributors have complained about it yet,
> 
> Fedora ships arptables_jf, which does not use -o,-g, thus does not have
> a problem to report.
> 
> OpenSUSE shipped arptables_jf. I moved it back to arptables
> because the jf fork is in an uncertain state.
> 

Yeah, too bad Redhat was so mysterious about this. The command line
options aren't even compatible. I wouldn't mind if arptables_jf became
the version used by all distros, but I'll maintain my own version as
long as it's used. On my laptop arptables_jf version 0.0.8 segfaults
with -L (built from sources).
I'll remove the -o -g stuff in ebtables' and arptables' Makefiles :-)

cheers,
Bart


-- 
Bart De Schuymer
www.artinalgorithms.be

Re: arptables 0.0.3-4 build fixes

[Bart De Schuymer]

Jan Engelhardt wrote:

>> The warings that occur when using llu with 64-bit numbers should be
>> resolved with the PRIu64 macro. Could you fix that and resubmit, please?
>> Thanks.
> 
> Hmm, iptables is not using PRI either so it seemed like the preferred
> approach.

I've committed the rest of your patches too, altered where needed to use
the PRIu64 macro.

cheers,
Bart


-- 
Bart De Schuymer
www.artinalgorithms.be