Re: [PATCH] iptables-save: Exit with error if unable to open proc file

Re: [PATCH] iptables-save: Exit with error if unable to open proc file

[thomas]

On Wed, 18 Jan 2017 14:32:30 +0100, Florian Westphal <fw@strlen.de> said:
> static const char filename[] =

Done.

> iptables uses kernel coding style, so
>
> 	if (errno == ENOENT)
> 		return ret;

Gotcha. Making the code unambiguously worse, then. (cough, goto fail, cough)
Done.

> Looks like your mua mangled the patch and broke long lines.
> Can you send with git-send-email?

Using a better client this time.


commit 0d18c3e9488ac2e36a5c5ecccce93de795f6fe25
Author: Thomas Habets <habets@google.com>
Date:   Wed Jan 18 13:46:54 2017 +0000

    iptables-save: Exit with error if unable to open proc file

diff --git a/iptables/ip6tables-save.c b/iptables/ip6tables-save.c
index f35e921..053413a 100644
--- a/iptables/ip6tables-save.c
+++ b/iptables/ip6tables-save.c
@@ -35,10 +35,16 @@ static int for_each_table(int (*func)(const char
*tablename))
 	int ret = 1;
 	FILE *procfile = NULL;
 	char tablename[XT_TABLE_MAXNAMELEN+1];
-
-	procfile = fopen("/proc/net/ip6_tables_names", "re");
-	if (!procfile)
-		return ret;
+	static const char filename[] = "/proc/net/ip6_tables_names";
+
+	procfile = fopen(filename, "re");
+	if (!procfile) {
+		if (errno == ENOENT)
+			return ret;
+		fprintf(stderr, "Failed to list table names in %s: %s\n",
+		        filename, strerror(errno));
+		exit(1);
+	}

 	while (fgets(tablename, sizeof(tablename), procfile)) {
 		if (tablename[strlen(tablename) - 1] != '\n')
diff --git a/iptables/iptables-save.c b/iptables/iptables-save.c
index 238f368..e8ae9c6 100644
--- a/iptables/iptables-save.c
+++ b/iptables/iptables-save.c
@@ -33,10 +33,16 @@ static int for_each_table(int (*func)(const char
*tablename))
 	int ret = 1;
 	FILE *procfile = NULL;
 	char tablename[XT_TABLE_MAXNAMELEN+1];
-
-	procfile = fopen("/proc/net/ip_tables_names", "re");
-	if (!procfile)
-		return ret;
+	static const char filename[] = "/proc/net/ip_tables_names";
+
+	procfile = fopen(filename, "re");
+	if (!procfile) {
+		if (errno == ENOENT)
+			return ret;
+		fprintf(stderr, "Failed to list table names in %s: %s\n",
+		        filename, strerror(errno));
+		exit(1);
+	}

 	while (fgets(tablename, sizeof(tablename), procfile)) {
 		if (tablename[strlen(tablename) - 1] != '\n')

Re: [PATCH] iptables-save: Exit with error if unable to open proc file

[Florian Westphal]

thomas@habets.se <thomas@habets.se> wrote:
> diff --git a/iptables/ip6tables-save.c b/iptables/ip6tables-save.c
> index f35e921..053413a 100644
> --- a/iptables/ip6tables-save.c
> +++ b/iptables/ip6tables-save.c
> @@ -35,10 +35,16 @@ static int for_each_table(int (*func)(const char
> *tablename))

Still a linewrap here, rest was fine so I fixed this up and applied
the patch, thanks!

I did not notce on 1st review but a 'Signed-off-by' line would be
good for future submissions.

Re: [PATCH] iptables-save: Exit with error if unable to open proc file

[thomas]

On Wed, 18 Jan 2017 15:07:17 +0100, Florian Westphal <fw@strlen.de> said:
> Still a linewrap here, rest was fine so I fixed this up and applied
> the patch, thanks!

Curious. My outgoing history claims it's a single line. I'll have to
look into this.

> I did not notce on 1st review but a 'Signed-off-by' line would be
> good for future submissions.

I'll keep it in mind next time. Thanks!

[PATCH] iptables-save: Exit with error if unable to open proc file

[Thomas Habets]

I sent this before without being subscribed, but it doesn't seem to
have reached the archives so now retrying while subscribed.

If you're not root, then iptables-save silently fails (both to stderr
and exit code). This patch fixes that.
--


Author: Thomas Habets <habets@google.com>
Date:   Tue Jan 17 14:22:25 2017 +0000

    iptables-save: Exit with error if unable to open proc file

diff --git a/iptables/ip6tables-save.c b/iptables/ip6tables-save.c
index f35e921..5097406 100644
--- a/iptables/ip6tables-save.c
+++ b/iptables/ip6tables-save.c
@@ -35,10 +35,16 @@ static int for_each_table(int (*func)(const char
*tablename))
        int ret = 1;
        FILE *procfile = NULL;
        char tablename[XT_TABLE_MAXNAMELEN+1];
+       const char *filename = "/proc/net/ip6_tables_names";

-       procfile = fopen("/proc/net/ip6_tables_names", "re");
-       if (!procfile)
-               return ret;
+       procfile = fopen(filename, "re");
+       if (!procfile) {
+               if (errno == ENOENT) {
+                       return ret;
+               }
+               fprintf(stderr, "Failed to list table names in %s:
%s\n", filename, strerror(errno));
+               exit(1);
+       }

        while (fgets(tablename, sizeof(tablename), procfile)) {
                if (tablename[strlen(tablename) - 1] != '\n')
diff --git a/iptables/iptables-save.c b/iptables/iptables-save.c
index 238f368..47d5378 100644
--- a/iptables/iptables-save.c
+++ b/iptables/iptables-save.c
@@ -33,10 +33,16 @@ static int for_each_table(int (*func)(const char
*tablename))
        int ret = 1;
        FILE *procfile = NULL;
        char tablename[XT_TABLE_MAXNAMELEN+1];
+       const char *filename = "/proc/net/ip_tables_names";

-       procfile = fopen("/proc/net/ip_tables_names", "re");
-       if (!procfile)
-               return ret;
+       procfile = fopen(filename, "re");
+       if (!procfile) {
+               if (errno == ENOENT) {
+                       return ret;
+               }
+               fprintf(stderr, "Failed to list table names in %s:
%s\n", filename, strerror(errno));
+               exit(1);
+       }

        while (fgets(tablename, sizeof(tablename), procfile)) {
                if (tablename[strlen(tablename) - 1] != '\n')

-- 
typedef struct me_s {
 char name[]      = { "Thomas Habets" };
 char email[]     = { "thomas@habets.se" };
 char kernel[]    = { "Linux" };
 char *pgpKey[]   = { "http://www.habets.pp.se/pubkey.txt" };
 char pgp[] = { "9907 8698 8A24 F52F 1C2E  87F6 39A4 9EEA 460A 0169" };
 char coolcmd[]   = { "echo '. ./_&. ./_'>_;. ./_" };
} me_t;

Re: [PATCH] iptables-save: Exit with error if unable to open proc file

[Florian Westphal]

Thomas Habets <thomas@habets.se> wrote:
> I sent this before without being subscribed, but it doesn't seem to
> have reached the archives so now retrying while subscribed.

Weird, its an open list (no subscribe required).

> If you're not root, then iptables-save silently fails (both to stderr
> and exit code). This patch fixes that.

Thanks, patch looks good but

> --- a/iptables/ip6tables-save.c
> +++ b/iptables/ip6tables-save.c
> @@ -35,10 +35,16 @@ static int for_each_table(int (*func)(const char
> *tablename))
>         int ret = 1;
>         FILE *procfile = NULL;
>         char tablename[XT_TABLE_MAXNAMELEN+1];
> +       const char *filename = "/proc/net/ip6_tables_names";

static const char filename[] = "

> -       procfile = fopen("/proc/net/ip6_tables_names", "re");
> -       if (!procfile)
> -               return ret;
> +       procfile = fopen(filename, "re");
> +       if (!procfile) {
> +               if (errno == ENOENT) {
> +                       return ret;
> +               }

iptables uses kernel coding style, so

	if (errno == ENOENT)
		return ret;

(no { } )

> +               fprintf(stderr, "Failed to list table names in %s:
> %s\n", filename, strerror(errno));
> +               exit(1);

Looks like your mua mangled the patch and broke long lines.
Can you send with git-send-email?

Otherwise see Documentation/email-clients.txt
in the linux kernel sources.