From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick McHardy Subject: Re: [PATCH 3/3] iptables-edit: introduces iptables-edit cli tool Date: Tue, 06 Nov 2007 01:13:09 +0100 Message-ID: <472FB195.6090202@trash.net> References: <472E604D.5090901@endian.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: netfilter-devel@vger.kernel.org To: Peter Warasin Return-path: Received: from stinky.trash.net ([213.144.137.162]:47933 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754786AbXKFAN0 (ORCPT ); Mon, 5 Nov 2007 19:13:26 -0500 In-Reply-To: <472E604D.5090901@endian.com> Sender: netfilter-devel-owner@vger.kernel.org List-Id: netfilter-devel.vger.kernel.org Peter Warasin wrote: The patch has some stylistic problems, see below for a few details. I suggest to run it through Lindent. > Index: iptables-edit.c > =================================================================== > --- /dev/null > +++ iptables-edit.c > @@ -0,0 +1,259 @@ > +/* Code to apply iptables rules on an iptables dump file generated by iptables-save. */ > +/* (C) 2007 by Peter Warasin > + * based on previous code from Rusty Russell > + * and Harald Welte > + * > + * This code is distributed under the terms of GNU GPL v2 > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include "libiptc/libiptc.h" > +#include "iptables.h" > +#include "iptables-dump.h" > + > +int binary = 0, counters = 0, verbose = 0; > +char *modprobeparam = 0; > +char *dumpfile = 0; > +int commandargc = 0; > +char *commandargv[255]; > + > +static struct option options[] = { > + { "binary", 0, 0, 'b' }, > + { "counters", 0, 0, 'c' }, > + { "verbose", 0, 0, 'v' }, > + { "help", 0, 0, 'h' }, > + { "modprobe", 1, 0, 'M'}, > + { "dump-file", 0, 0, 'i' }, > Broken whitespace. Please also use NULL instead of 0 for the third argument. > + { 0 } > +}; > + > +struct handle_list_t { > + char tablename[IPT_TABLE_MAXNAMELEN + 1]; > + iptc_handle_t handle; > + struct handle_list_t *next; > +}; > +struct handle_list_t *table_handles = NULL; > + > +static void print_usage(const char *name, const char *version) __attribute__((noreturn)); > + > +static void print_usage(const char *name, const char *version) > +{ > + fprintf(stderr, "Usage: %s [-b] [-c] [-v] [-h] [-M] <-i>\n" > + " [ --binary ]\n" > + " [ --counters ]\n" > + " [ --verbose ]\n" > + " [ --help ]\n" > + " [ --modprobe=]\n" > + " [ --dump-file=]\n", name); > + exit(1); > +} > + > +void add_handle(const char *tablename, iptc_handle_t handle) { > opening parens goes on a new line. > + struct handle_list_t *tmp; > + tmp = (struct handle_list_t *) malloc(sizeof(struct handle_list_t)); > + strncpy(tmp->tablename, tablename, IPT_TABLE_MAXNAMELEN); > + tmp->tablename[IPT_TABLE_MAXNAMELEN] = '\0'; > + tmp->handle = handle; > + tmp->next = table_handles; > + table_handles = tmp; > +} > + > +iptc_handle_t get_handle(const char *tablename) { > + iptc_handle_t handle; > + struct handle_list_t *i = NULL; > unnecessary initialization > + if (tablename == NULL) > + return NULL; > + for (i = table_handles; i; i = i->next) { > + if (! i) break; > newline please. > + if (strcmp(i->tablename, tablename) == 0) > + return i->handle; > + } > + > + handle = iptc_init(tablename); > + add_handle(tablename, handle); > + return handle; > +} > + > +static int for_each_table(int (*func)(const char *tablename)) > +{ > + int ret = 1; > + FILE *procfile = NULL; > + char tablename[IPT_TABLE_MAXNAMELEN+1]; > + > + procfile = fopen("/proc/net/ip_tables_names", "r"); > + if (!procfile) > + return 0; > + > + while (fgets(tablename, sizeof(tablename), procfile)) { > + if (tablename[strlen(tablename) - 1] != '\n') > + exit_error(OTHER_PROBLEM, > + "Badly formed tablename `%s'\n", > + tablename); > + tablename[strlen(tablename) - 1] = '\0'; > + ret &= func(tablename); > + } > + > + return ret; > +} > + > +int restore_from_file(const char *tablename) { > + iptc_handle_t handle = get_handle(tablename); > + if (verbose) > + fprintf(stderr, "Restoring table '%s'\n", tablename); > + > + if (! handle) { > + fprintf(stderr, "Could not get netfilter handle for table '%s' while restoring\n", tablename); > + return 0; > + } > + return restore_dump(tablename, handle, modprobeparam, dumpfile, binary, counters, verbose, 0, 1); > Please break lines at 80 characters. > +} > + > +int save_handles(const char *tablename) { > + iptc_handle_t handle = get_handle(tablename); > + if (verbose) > + fprintf(stderr, "Saving table '%s'\n", tablename); > + if (! handle) { > + fprintf(stderr, "Could not get netfilter handle for table '%s' while saving\n", tablename); > + return 0; > + } > + return create_dump(tablename, handle, binary, counters); > +} > + > + > +/* function adding one argument to newargv, updating newargc > + * returns true if argument added, false otherwise */ > +static int add_argv(char *what) { > + if (what && ((commandargc + 1) < sizeof(commandargv)/sizeof(char *))) { > + commandargv[commandargc] = strdup(what); > + commandargc++; > + return 1; > + } else > + return 0; > +} > + > +static void free_argv(void) { > + int i; > + > + for (i = 0; i < commandargc; i++) { > + free(commandargv[i]); > + commandargv[i] = NULL; > + } > + commandargc = 0; > +} > + > + > +#ifdef IPTABLES_MULTI > +int > +iptables_edit_main(int argc, char *argv[]) > +#else > +int > +main(int argc, char *argv[]) > +#endif > +{ > + int c; > + int ret = 0; > + char buffer[10240]; > + int i=0; > + > + program_name = "iptables"; > + program_version = IPTABLES_VERSION; > + > + lib_dir = getenv("IPTABLES_LIB_DIR"); > + if (!lib_dir) > + lib_dir = IPT_LIB_DIR; > + > +#ifdef NO_SHARED_LIBS > + init_extensions(); > +#endif > + > + while ((c = getopt_long(argc, argv, "bcvhM:i:", options, NULL)) != -1) { > + switch (c) { > + case 'b': > + binary = 1; > + break; > + > + case 'c': > + counters = 1; > + break; > + case 'v': > + verbose = 1; > + break; > + case 'h': > + print_usage("iptables-edit", > + IPTABLES_VERSION); > + break; > + case 'M': > + modprobeparam = optarg; > + break; > + case 'i': > + dumpfile = optarg; > + break; > + > + } > + } > + > + if (optind < argc) { > + fprintf(stderr, "Unknown arguments found on commandline\n"); > + exit(1); > + } > + > + if (! dumpfile) { > + fprintf(stderr, "No dump file (-i) specified!\n"); > + exit(1); > + } > + > + if ((ret = for_each_table(restore_from_file)) != 0) > + return ret; > + > + if (verbose) > + fprintf(stderr, "Accept commands\n"); > + > + /* Grab standard input. */ > + while (fgets(buffer, sizeof(buffer), stdin)) { > + char *token = ""; > + iptc_handle_t handle = NULL; > + char *thistable = "filter"; > + > + i++; > + buffer[strlen(buffer)-1] = '\0'; > + if (buffer[0] == '#') > + continue; > + if (verbose) > + fprintf(stderr, "Line %d: Process command '%s'\n", i, buffer); > + > + if ((token = strtok(buffer, " \t\n")) == NULL) > + continue; > + free_argv(); > + add_argv(token); > + while ((token = strtok(NULL, " \t\n")) != NULL) { > + add_argv(token); > + } > + > + if ((commandargv[1] != NULL) && strcmp(commandargv[1], "-t") == 0) { > + if (commandargv[2] == NULL) { > + fprintf(stderr, "Line %d: -t parameter needs an argument!\n", i); > + return 1; > + } > + thistable = commandargv[2]; > + } > + > + handle = get_handle(thistable); > + if (handle == NULL) { > + fprintf(stderr, "Line %d: Could not get netfilter handle for table '%s' while performing command\n", i, thistable); > + return 1; > + } > + > + if (! do_command(commandargc, commandargv, > + &thistable, &handle)) { > + > + fprintf(stderr, "Line %d: Command failed: %s\n", i, iptc_strerror(errno)); > + return 1; > + } > + } > + > + return ! for_each_table(save_handles); > +} >