From patchwork Mon Jun 18 08:11:46 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 930735 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 418P2S5l2Mz9s2R for ; Mon, 18 Jun 2018 18:12:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933129AbeFRIL7 (ORCPT ); Mon, 18 Jun 2018 04:11:59 -0400 Received: from orbyte.nwl.cc ([151.80.46.58]:39148 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933079AbeFRIL6 (ORCPT ); Mon, 18 Jun 2018 04:11:58 -0400 Received: from localhost ([::1]:54418 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.90_1) (envelope-from ) id 1fUpGV-0007Fa-45; Mon, 18 Jun 2018 10:11:55 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org, Laura Garcia Liebana , Eric Leblond , Arturo Borrero Gonzalez Subject: [nft PATCH v2] libnftables: Simplify nft_run_cmd_from_buffer footprint Date: Mon, 18 Jun 2018 10:11:46 +0200 Message-Id: <20180618081146.4301-1-phil@nwl.cc> X-Mailer: git-send-email 2.17.0 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org 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 Cc: Laura Garcia Liebana Cc: Eric Leblond Cc: Arturo Borrero Gonzalez Signed-off-by: Phil Sutter --- 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(-) 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 = "", }; -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 = {