Re: [PATCH 3/3] iptables-edit: introduces iptables-edit cli tool

[Patrick McHardy <kaber@trash.net>]

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 <peter@endian.com>
> + * based on previous code from Rusty Russell <rusty@linuxcare.com.au>
> + * and Harald Welte <laforge@gnumonks.org>
> + *
> + * This code is distributed under the terms of GNU GPL v2
> + *
> + */
> +#include <getopt.h>
> +#include <sys/errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#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=<command>]\n"
> +                        "          [ --dump-file=<DUMPFILE>]\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);
> +}
>