diff mbox series

[nft,v2] libnftables: Simplify nft_run_cmd_from_buffer footprint

Message ID 20180618081146.4301-1-phil@nwl.cc
State Accepted
Delegated to: Pablo Neira
Headers show
Series [nft,v2] libnftables: Simplify nft_run_cmd_from_buffer footprint | expand

Commit Message

Phil Sutter June 18, 2018, 8:11 a.m. UTC 
With libnftables documentation being upstream and one confirmed external
user (nftlb), time to break the API!

First of all, the command buffer passed to nft_run_cmd_from_buffer may
(and should) be const. One should consider it a bug if that function
ever changed it's content.

On the other hand, there is no point in passing the buffer's length as
separate argument: NULL bytes are not expected to occur in the input, so
it is safe to rely upon strlen(). Also, the actual parsers don't require
a buffer length passed to them, either. The only use-case for it is when
reallocating the buffer to append a final newline character, there
strlen() is perfectly sufficient.

Suggested-by: Harald Welte <laforge@gnumonks.org>
Cc: Laura Garcia Liebana <nevola@gmail.com>
Cc: Eric Leblond <eric@regit.org>
Cc: Arturo Borrero Gonzalez <arturo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Add -version-info to libnftables LDFLAGS to bump library interface
  version from 0 to 1.
---
 doc/libnftables.adoc           |  9 ++++-----
 include/json.h                 |  5 +++--
 include/nftables/libnftables.h |  2 +-
 py/nftables.py                 |  4 ++--
 src/Makefile.am                |  1 +
 src/cli.c                      |  2 +-
 src/libnftables.c              | 14 ++++++--------
 src/main.c                     |  2 +-
 src/parser_json.c              |  2 +-
 9 files changed, 20 insertions(+), 21 deletions(-)

Comments

Pablo Neira Ayuso June 18, 2018, 9:18 a.m. UTC | #1 
On Mon, Jun 18, 2018 at 10:11:46AM +0200, Phil Sutter wrote:
[...]
> First of all, the command buffer passed to nft_run_cmd_from_buffer may
> (and should) be const. One should consider it a bug if that function
> ever changed it's content.

Actually applied this v2 :-).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index c947ef37f8f1d..adfc94205a8a5 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -53,8 +53,7 @@  const char *nft_ctx_get_error_buffer(struct nft_ctx* '\*ctx'*);
 int nft_ctx_add_include_path(struct nft_ctx* '\*ctx'*, const char* '\*path'*);
 void nft_ctx_clear_include_paths(struct nft_ctx* '\*ctx'*);
 
-int nft_run_cmd_from_buffer(struct nft_ctx* '\*nft'*,
-			    char* '\*buf'*, size_t* 'buflen'*);
+int nft_run_cmd_from_buffer(struct nft_ctx* '\*nft'*, const char* '\*buf'*);
 int nft_run_cmd_from_filename(struct nft_ctx* '\*nft'*,
 			      const char* '\*filename'*);*
 
@@ -244,7 +243,7 @@  The *nft_ctx_clear_include_paths*() function removes all include paths, even the
 === nft_run_cmd_from_buffer() and nft_run_cmd_from_filename()
 These functions perform the actual work of parsing user input into nftables commands and executing them.
 
-The *nft_run_cmd_from_buffer*() function passes the command(s) contained in 'buf' with size 'buflen' to the library, respecting settings and state in 'nft'.
+The *nft_run_cmd_from_buffer*() function passes the command(s) contained in 'buf' (which must be null-terminated) to the library, respecting settings and state in 'nft'.
 
 The *nft_run_cmd_from_filename*() function passes the content of 'filename' to the library, respecting settings and state in 'nft'.
 
@@ -272,7 +271,7 @@  int main(void)
 
 	while (1) {
 		if (nft_ctx_buffer_output(nft) ||
-		    nft_run_cmd_from_buffer(nft, list_cmd, strlen(list_cmd))) {
+		    nft_run_cmd_from_buffer(nft, list_cmd)) {
 			rc = 1;
 			break;
 		}
@@ -300,7 +299,7 @@  int main(void)
 		if (buf[0] == 'q' && buf[1] == '\0')
 			break;
 
-		if (nft_run_cmd_from_buffer(nft, buf, strlen(buf))) {
+		if (nft_run_cmd_from_buffer(nft, buf)) {
 			rc = 1;
 			break;
 		}
diff --git a/include/json.h b/include/json.h
index 7d2af12907d23..ab898333af588 100644
--- a/include/json.h
+++ b/include/json.h
@@ -79,7 +79,7 @@  json_t *connlimit_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
 
 int do_command_list_json(struct netlink_ctx *ctx, struct cmd *cmd);
 
-int nft_parse_json_buffer(struct nft_ctx *nft, char *buf, size_t buflen,
+int nft_parse_json_buffer(struct nft_ctx *nft, const char *buf,
 			  struct list_head *msgs, struct list_head *cmds);
 int nft_parse_json_filename(struct nft_ctx *nft, const char *filename,
 			    struct list_head *msgs, struct list_head *cmds);
@@ -172,11 +172,12 @@  static inline int do_command_list_json(struct netlink_ctx *ctx, struct cmd *cmd)
 }
 
 static inline int
-nft_parse_json_buffer(struct nft_ctx *nft, char *buf, size_t buflen,
+nft_parse_json_buffer(struct nft_ctx *nft, const char *buf,
 		      struct list_head *msgs, struct list_head *cmds)
 {
 	return -EINVAL;
 }
+
 static inline int
 nft_parse_json_filename(struct nft_ctx *nft, const char *filename,
 			struct list_head *msgs, struct list_head *cmds)
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 4bfdaf9a26885..13ec392735816 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -71,7 +71,7 @@  const char *nft_ctx_get_error_buffer(struct nft_ctx *ctx);
 int nft_ctx_add_include_path(struct nft_ctx *ctx, const char *path);
 void nft_ctx_clear_include_paths(struct nft_ctx *ctx);
 
-int nft_run_cmd_from_buffer(struct nft_ctx *nft, char *buf, size_t buflen);
+int nft_run_cmd_from_buffer(struct nft_ctx *nft, const char *buf);
 int nft_run_cmd_from_filename(struct nft_ctx *nft, const char *filename);
 
 #endif /* LIB_NFTABLES_H */
diff --git a/py/nftables.py b/py/nftables.py
index 47ff14afc9741..e6e66fb776be7 100644
--- a/py/nftables.py
+++ b/py/nftables.py
@@ -100,7 +100,7 @@  class Nftables:
 
         self.nft_run_cmd_from_buffer = lib.nft_run_cmd_from_buffer
         self.nft_run_cmd_from_buffer.restype = c_int
-        self.nft_run_cmd_from_buffer.argtypes = [c_void_p, c_char_p, c_int]
+        self.nft_run_cmd_from_buffer.argtypes = [c_void_p, c_char_p]
 
         self.nft_ctx_free = lib.nft_ctx_free
         lib.nft_ctx_free.argtypes = [c_void_p]
@@ -267,7 +267,7 @@  class Nftables:
         output -- a string containing output written to stdout
         error  -- a string containing output written to stderr
         """
-        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline, len(cmdline))
+        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline)
         output = self.nft_ctx_get_output_buffer(self.__ctx)
         error = self.nft_ctx_get_error_buffer(self.__ctx)
 
diff --git a/src/Makefile.am b/src/Makefile.am
index a4ad8cb31236b..92ac61a6044e6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -70,6 +70,7 @@  libparser_la_CFLAGS = ${AM_CFLAGS} \
 		      -Wno-redundant-decls
 
 libnftables_la_LIBADD = ${LIBMNL_LIBS} ${LIBNFTNL_LIBS} libparser.la
+libnftables_la_LDFLAGS = -version-info 1:0:0
 
 if BUILD_MINIGMP
 noinst_LTLIBRARIES += libminigmp.la
diff --git a/src/cli.c b/src/cli.c
index 241ea0105512f..ca3869abe335f 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -110,7 +110,7 @@  static void cli_complete(char *line)
 	if (hist == NULL || strcmp(hist->line, line))
 		add_history(line);
 
-	nft_run_cmd_from_buffer(cli_nft, line, strlen(line) + 1);
+	nft_run_cmd_from_buffer(cli_nft, line);
 	xfree(line);
 }
 
diff --git a/src/libnftables.c b/src/libnftables.c
index 760deecf2b899..77012e92d7495 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -396,7 +396,7 @@  static const struct input_descriptor indesc_cmdline = {
 	.name	= "<cmdline>",
 };
 
-static int nft_parse_bison_buffer(struct nft_ctx *nft, char *buf, size_t buflen,
+static int nft_parse_bison_buffer(struct nft_ctx *nft, const char *buf,
 				  struct list_head *msgs, struct list_head *cmds)
 {
 	struct cmd *cmd;
@@ -438,23 +438,21 @@  static int nft_parse_bison_filename(struct nft_ctx *nft, const char *filename,
 	return 0;
 }
 
-int nft_run_cmd_from_buffer(struct nft_ctx *nft, char *buf, size_t buflen)
+int nft_run_cmd_from_buffer(struct nft_ctx *nft, const char *buf)
 {
 	struct cmd *cmd, *next;
 	LIST_HEAD(msgs);
 	LIST_HEAD(cmds);
-	size_t nlbuflen;
 	char *nlbuf;
 	int rc = -EINVAL;
 
-	nlbuflen = max(buflen + 1, strlen(buf) + 2);
-	nlbuf = xzalloc(nlbuflen);
-	snprintf(nlbuf, nlbuflen, "%s\n", buf);
+	nlbuf = xzalloc(strlen(buf) + 2);
+	sprintf(nlbuf, "%s\n", buf);
 
 	if (nft->output.json)
-		rc = nft_parse_json_buffer(nft, nlbuf, nlbuflen, &msgs, &cmds);
+		rc = nft_parse_json_buffer(nft, nlbuf, &msgs, &cmds);
 	if (rc == -EINVAL)
-		rc = nft_parse_bison_buffer(nft, nlbuf, nlbuflen, &msgs, &cmds);
+		rc = nft_parse_bison_buffer(nft, nlbuf, &msgs, &cmds);
 	if (rc)
 		goto err;
 
diff --git a/src/main.c b/src/main.c
index f36159744744a..b2966a41e14f6 100644
--- a/src/main.c
+++ b/src/main.c
@@ -279,7 +279,7 @@  int main(int argc, char * const *argv)
 			if (i + 1 < argc)
 				strcat(buf, " ");
 		}
-		rc = !!nft_run_cmd_from_buffer(nft, buf, len);
+		rc = !!nft_run_cmd_from_buffer(nft, buf);
 	} else if (filename != NULL) {
 		rc = !!nft_run_cmd_from_filename(nft, filename);
 	} else if (interactive) {
diff --git a/src/parser_json.c b/src/parser_json.c
index 40557a3e62d1f..80e3aafe08447 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3125,7 +3125,7 @@  static int __json_parse(struct json_ctx *ctx, json_t *root)
 
 static json_t *cur_root = NULL;
 
-int nft_parse_json_buffer(struct nft_ctx *nft, char *buf, size_t buflen,
+int nft_parse_json_buffer(struct nft_ctx *nft, const char *buf,
 			  struct list_head *msgs, struct list_head *cmds)
 {
 	struct json_ctx ctx = {