[PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Peter Warasin]

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


-- 
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: peter@endian.com

[-- Attachment #2: iptables-edit-p1.patch --]
[-- Type: text/x-patch, Size: 1711 bytes --]

adds --table to iptables-restore which allows to restore only the
supplied table

Signed-off-by: Peter Warasin <peter@endian.com>
---
 iptables-restore.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Index: iptables-restore.c
===================================================================
--- iptables-restore.c.orig
+++ iptables-restore.c
@@ -33,6 +33,7 @@
 	{ "help", 0, 0, 'h' },
 	{ "noflush", 0, 0, 'n'},
 	{ "modprobe", 1, 0, 'M'},
+	{ "table", 1, 0, 'T'},
 	{ 0 }
 };
 
@@ -47,6 +48,7 @@
 			"	   [ --test ]\n"
 			"	   [ --help ]\n"
 			"	   [ --noflush ]\n"
+			"	   [ --table=<TABLE> ]\n"
 		        "          [ --modprobe=<command>]\n", name);
 		
 	exit(1);
@@ -123,6 +125,7 @@
 	FILE *in;
 	const char *modprobe = 0;
 	int in_table = 0, testing = 0;
+	const char *tablename = 0;
 
 	program_name = "iptables-restore";
 	program_version = IPTABLES_VERSION;
@@ -136,7 +139,7 @@
 	init_extensions();
 #endif
 
-	while ((c = getopt_long(argc, argv, "bcvthnM:", options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "bcvthnM:T:", options, NULL)) != -1) {
 		switch (c) {
 			case 'b':
 				binary = 1;
@@ -160,6 +163,9 @@
 			case 'M':
 				modprobe = optarg;
 				break;
+			case 'T':
+				tablename = optarg;
+				break;
 		}
 	}
 	
@@ -212,6 +218,8 @@
 			strncpy(curtable, table, IPT_TABLE_MAXNAMELEN);
 			curtable[IPT_TABLE_MAXNAMELEN] = '\0';
 
+			if (tablename && (strcmp(tablename, table) != 0))
+				continue;
 			if (handle)
 				iptc_free(&handle);
 
@@ -438,6 +446,8 @@
 
 			free_argv();
 		}
+		if (tablename && (strcmp(tablename, curtable) != 0))
+			continue;
 		if (!ret) {
 			fprintf(stderr, "%s: line %u failed\n",
 					program_name, line);

[-- Attachment #3: peter.vcf --]
[-- Type: text/x-vcard, Size: 279 bytes --]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia
email;internet:peter@endian.com
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Patrick McHardy]

Peter Warasin wrote:

> adds --table to iptables-restore which allows to restore only the
> supplied table

Applied, thanks Peter.

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Jan Engelhardt]

On Nov 5 2007 01:13, Peter Warasin wrote:

>Date: Mon, 05 Nov 2007 01:13:53 +0100
>From: Peter Warasin <peter@endian.com>
>To:  <netfilter-devel@vger.kernel.org>
>Subject: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to
>    iptables-restore
>
>
>-- 
>:: e n d i a n
>:: open source - open minds
>
>:: peter warasin
>:: http://www.endian.com   :: peter@endian.com
>

Well, please send patches inline so I can reply to something.


|adds --table to iptables-restore which allows to restore only the
|supplied table
|
|Signed-off-by: Peter Warasin <peter@endian.com>
|---
| iptables-restore.c |   12 +++++++++++-
| 1 file changed, 11 insertions(+), 1 deletion(-)
|
|Index: iptables-restore.c
|===================================================================
|--- iptables-restore.c.orig
|+++ iptables-restore.c
|@@ -33,6 +33,7 @@
| 	{ "help", 0, 0, 'h' },
| 	{ "noflush", 0, 0, 'n'},
| 	{ "modprobe", 1, 0, 'M'},
|+	{ "table", 1, 0, 'T'},
| 	{ 0 }
| };
| 
|@@ -47,6 +48,7 @@
| 			"	   [ --test ]\n"
| 			"	   [ --help ]\n"
| 			"	   [ --noflush ]\n"
|+			"	   [ --table=<TABLE> ]\n"
| 		        "          [ --modprobe=<command>]\n", name);
| 		
| 	exit(1);
|@@ -123,6 +125,7 @@
| 	FILE *in;
| 	const char *modprobe = 0;
| 	int in_table = 0, testing = 0;
|+	const char *tablename = 0;

Use NULL instead.

| 
| 	program_name = "iptables-restore";
| 	program_version = IPTABLES_VERSION;
|@@ -136,7 +139,7 @@
| 	init_extensions();
| #endif
| 
|-	while ((c = getopt_long(argc, argv, "bcvthnM:", options, NULL)) != -1) {
|+	while ((c = getopt_long(argc, argv, "bcvthnM:T:", options, NULL)) != -1) {

Use -t instead, that would go in line with the -t from iptables.

| 		switch (c) {
| 			case 'b':
| 				binary = 1;
|@@ -160,6 +163,9 @@
| 			case 'M':
| 				modprobe = optarg;
| 				break;
|+			case 'T':
|+				tablename = optarg;
|+				break;
| 		}
| 	}
| 	
|@@ -212,6 +218,8 @@
| 			strncpy(curtable, table, IPT_TABLE_MAXNAMELEN);
| 			curtable[IPT_TABLE_MAXNAMELEN] = '\0';
| 
|+			if (tablename && (strcmp(tablename, table) != 0))
|+				continue;

drop extra ()

| 			if (handle)
| 				iptc_free(&handle);
| 
|@@ -438,6 +446,8 @@
| 
| 			free_argv();
| 		}
|+		if (tablename && (strcmp(tablename, curtable) != 0))
|+			continue;
| 		if (!ret) {
| 			fprintf(stderr, "%s: line %u failed\n",
| 					program_name, line);

same

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Peter Warasin]

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

Hi Jan

Jan Engelhardt wrote:
> Well, please send patches inline so I can reply to something.

Ok,..  I think i eventually have to upgrade my thunderbird, since i have
no option for sending inline attachments :-O


> |adds --table to iptables-restore which allows to restore only the
> |supplied table

This patch is already applied. Should i fix your suggestions or leave it
as it is?



> |-	while ((c = getopt_long(argc, argv, "bcvthnM:", options, NULL)) != -1) {
> |+	while ((c = getopt_long(argc, argv, "bcvthnM:T:", options, NULL)) != -1) {
> 
> Use -t instead, that would go in line with the -t from iptables.

-t is already --test, therefore i used -T
I suggest changing --test to --dry-run (-d), so -t will turn free for
--table, what do you think?

peter

-- 
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: peter@endian.com

[-- Attachment #2: peter.vcf --]
[-- Type: text/x-vcard, Size: 279 bytes --]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia
email;internet:peter@endian.com
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Jan Engelhardt]

On Nov 7 2007 14:44, Peter Warasin wrote:
>Jan Engelhardt wrote:
>
>> |adds --table to iptables-restore which allows to restore only the
>> |supplied table
>
>This patch is already applied. Should i fix your suggestions or leave it
>as it is?

Try changing, it's just the Subversion thing.

>> |-	while ((c = getopt_long(argc, argv, "bcvthnM:", options, NULL)) != -1) {
>> |+	while ((c = getopt_long(argc, argv, "bcvthnM:T:", options, NULL)) != -1) {
>> 
>> Use -t instead, that would go in line with the -t from iptables.
>
>-t is already --test, therefore i used -T
>I suggest changing --test to --dry-run (-d), so -t will turn free for
>--table, what do you think?

Is a dry-run option really needed? iptables-edit does not commit
any tables to the kernel, so the program in itself is already
in dry-run mode. If the resulting ruleset that iptables-edit will
generate is not loadable (e.g. xt_somemodule missing), then
iptables-restore should turn that up when trying to load the
bogus ruleset.

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Peter Warasin]

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

Jan Engelhardt wrote:
>> This patch is already applied. Should i fix your suggestions or leave it
>> as it is?
> Try changing, it's just the Subversion thing.

Ok, i will resend later.


> Is a dry-run option really needed? iptables-edit does not commit

Attention, this is a new option of iptables-restore, not of iptables-edit.
--test was already implemented within iptables-restore.
So in order to use -t instead of -T it's necessary to rename --test to
for example --dry-run

peter

-- 
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: peter@endian.com

[-- Attachment #2: peter.vcf --]
[-- Type: text/x-vcard, Size: 279 bytes --]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia
email;internet:peter@endian.com
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Jan Engelhardt]

On Nov 7 2007 19:15, Peter Warasin wrote:
>
>> Is a dry-run option really needed? iptables-edit does not commit
>
>Attention, this is a new option of iptables-restore, not of iptables-edit.
>--test was already implemented within iptables-restore.
>So in order to use -t instead of -T it's necessary to rename --test to
>for example --dry-run

-t means table for:
	iptables
	iptables-save
	ip6tables
	ip6tables-save
	arptables
	ebtables

iptables-restore having -t for test really seems out of place.

That said, iptables-restore does not have a -t, according to its help text
("iptables-restore -h") and its manpage.

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Peter Warasin]

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

Hi Jan

Jan Engelhardt wrote:
> On Nov 7 2007 19:15, Peter Warasin wrote:
>>> Is a dry-run option really needed? iptables-edit does not commit
>> Attention, this is a new option of iptables-restore, not of iptables-edit.

> -t means table for:
[..]
> iptables-restore having -t for test really seems out of place.

i agree.


> That said, iptables-restore does not have a -t, according to its help text
> ("iptables-restore -h") and its manpage.

Thats not true. The current subversion code uses -t for --test. Probably
you confuse with iptables-save ?

http://svn.netfilter.org/cgi-bin/viewcvs.cgi/trunk/iptables/iptables-restore.c?rev=7087&view=markup
-------- snip -----------------------------------------------
			case 't':
				testing = 1;
				break;
-------- snap -----------------------------------------------

If you want me to change it to --dry-run, i will do that, so i can use
-t for --table.

peter

-- 
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: peter@endian.com

[-- Attachment #2: peter.vcf --]
[-- Type: text/x-vcard, Size: 279 bytes --]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia
email;internet:peter@endian.com
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Jan Engelhardt]

On Nov 7 2007 20:11, Peter Warasin wrote:
>> That said, iptables-restore does not have a -t, according to its help text
>> ("iptables-restore -h") and its manpage.
>
>Thats not true. The current subversion code uses -t for --test. Probably
>you confuse with iptables-save ?

actually, overlooked it :-/

>If you want me to change it to --dry-run, i will do that, so i can use
>-t for --table.

be prepared to hear the 'that breaks compatibilty' argument.
Though, since it's 1.4.0, I'd go for it :p

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Peter Warasin]

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

Hi Jan

Jan Engelhardt wrote:
> On Nov 7 2007 20:11, Peter Warasin wrote:
>> If you want me to change it to --dry-run, i will do that, so i can use
>> -t for --table.
> 
> be prepared to hear the 'that breaks compatibilty' argument.
> Though, since it's 1.4.0, I'd go for it :p

I imagine :)
Ok, i will try.

BTW: Is somewhere some sort of coding style documentation which i can
read in order to prevent such mistakes, or is it just the linux kernel
coding style?

Some of the malformed style you suggested to change are due to
copy&paste of current iptables code, which i used as reference in some
cases, so i am little confused now.

peter

-- 
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: peter@endian.com

[-- Attachment #2: peter.vcf --]
[-- Type: text/x-vcard, Size: 279 bytes --]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia
email;internet:peter@endian.com
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

Re: [PATCH 1/3] iptables-edit: iptables-edit: adds --table to iptables-restore

[Jan Engelhardt]

On Nov 7 2007 20:50, Peter Warasin wrote:
>Jan Engelhardt wrote:
>> On Nov 7 2007 20:11, Peter Warasin wrote:
>>> If you want me to change it to --dry-run, i will do that, so i can use
>>> -t for --table.
>> 
>> be prepared to hear the 'that breaks compatibilty' argument.
>> Though, since it's 1.4.0, I'd go for it :p
>
>I imagine :)
>Ok, i will try.
>
>BTW: Is somewhere some sort of coding style documentation which i can
>read in order to prevent such mistakes, or is it just the linux kernel
>coding style?

LKCS is the preferred one, yes.

>Some of the malformed style you suggested to change are due to
>copy&paste of current iptables code, which i used as reference in some
>cases, so i am little confused now.

Yup, there is a lot of crufty code. Like what, less than 1% of all
active developers of a project care about styling. Unfortunately for
netfilter, 1% of N is generally less than 1.