Re: [nft PATCH v3 2/2] src: add import command

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Mar 10, 2015 at 07:19:22PM +0100, Álvaro Neira Ayuso wrote:
> El 10/03/15 a las 11:21, Pablo Neira Ayuso escribió:
> >On Tue, Mar 10, 2015 at 11:04:18AM +0100, Alvaro Neira Ayuso wrote:
> >>diff --git a/src/rule.c b/src/rule.c
> >>index 8d76fd0..8c58a2b 100644
> >>--- a/src/rule.c
> >>+++ b/src/rule.c
> >>@@ -20,6 +20,7 @@
> >>  #include <rule.h>
> >>  #include <utils.h>
> >>  #include <netlink.h>
> >>+#include <mnl.h>
> >>
> >>  #include <libnftnl/common.h>
> >>  #include <libnftnl/ruleset.h>
> >>@@ -555,6 +556,21 @@ void export_free(struct export *e)
> >>  	xfree(e);
> >>  }
> >>
> >>+struct import *import_alloc(uint32_t format)
> >>+{
> >>+	struct import *import;
> >>+
> >>+	import = xmalloc(sizeof(struct import));
> >>+	import->format = format;
> >>+
> >>+	return import;
> >>+}
> >>+
> >>+void import_free(struct import *i)
> >>+{
> >>+	xfree(i);
> >>+}
> >>+
> >>  struct monitor *monitor_alloc(uint32_t format, uint32_t type, const char *event)
> >>  {
> >>  	struct monitor *mon;
> >>@@ -599,6 +615,9 @@ void cmd_free(struct cmd *cmd)
> >>  		case CMD_OBJ_MONITOR:
> >>  			monitor_free(cmd->monitor);
> >>  			break;
> >>+		case CMD_OBJ_IMPORT:
> >>+			import_free(cmd->import);
> >>+			break;
> >>  		case CMD_OBJ_EXPORT:
> >>  			export_free(cmd->export);
> >>  			break;
> >>@@ -1006,6 +1025,322 @@ static int do_command_describe(struct netlink_ctx *ctx, struct cmd *cmd)
> >>  	return 0;
> >>  }
> >>
> >>+struct ruleset_parse {
> >>+	struct netlink_ctx *nl_ctx;
> >>+	struct cmd *cmd;
> >>+};
> >>+
> >>+static int ruleset_parse_setelems(const struct nft_parse_ctx *ctx)
> >>+{
> >>+	const struct ruleset_parse *rp;
> >>+	struct nft_set *set;
> >>+	uint32_t cmd;
> >>+	int ret = -1;
> >>+
> >>+	set = nft_ruleset_ctx_get(ctx, NFT_RULESET_CTX_SET);
> >>+	rp = nft_ruleset_ctx_get(ctx, NFT_RULESET_CTX_DATA);
> >>+
> >>+	cmd = nft_ruleset_ctx_get_u32(ctx, NFT_RULESET_CTX_CMD);
> >>+	switch (cmd) {
> >>+	case NFT_CMD_ADD:
> >>+		ret = mnl_nft_setelem_batch_add(set, 0, rp->nl_ctx->seqnum);
> >>+		break;
> >>+	case NFT_CMD_DELETE:
> >>+		ret = mnl_nft_setelem_batch_del(set, 0, rp->nl_ctx->seqnum);
> >>+		break;
> >>+	default:
> >>+		errno = EOPNOTSUPP;
> >>+		break;
> >>+	}
> >>+
> >>+	if (ret < 0)
> >>+		netlink_io_error(rp->nl_ctx, &rp->cmd->location,
> >>+				 "Could not import set_elems: %s",
> >>+				 strerror(errno));
> >
> >I think rp->cmd->location is unset, so this will crash. Could you
> >validate this by forcing an error to make sure it works?
> 
> It's not unset. If we have an error, the location is in the import
> command. For example:
> 
> Error: Could not import set_elems: Invalid argument
> import json
> ^^^^^^^^^^^

Good, thanks.

BTW, please don't use developer jargon in the error messages, you
better say "Could not import set elements" instead of "set_elems".
--
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