All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>
Cc: Eric Leblond <eric@regit.org>,
	netfilter-devel@vger.kernel.org, Florian Westphal <fw@strlen.de>
Subject: Re: [nft PATCH v2 1/4] libnftables: Move library stuff out of main.c
Date: Tue, 24 Oct 2017 17:48:00 +0200	[thread overview]
Message-ID: <20171024154800.GA11705@salvia> (raw)
In-Reply-To: <20171023153319.13415-2-phil@nwl.cc>

Hi Phil,

Just follow up comments, several things that I think we can polish,
see below.

On Mon, Oct 23, 2017 at 05:33:16PM +0200, Phil Sutter wrote:
[...]
> diff --git a/include/nftables/nftables.h b/include/nftables/nftables.h
> new file mode 100644
> index 0000000000000..44d3e95d399e6
> --- /dev/null
> +++ b/include/nftables/nftables.h
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (c) 2017 Eric Leblond <eric@regit.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#ifndef LIB_NFTABLES_H
> +#define LIB_NFTABLES_H
> +
> +#define _GNU_SOURCE
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdbool.h>
> +
> +struct nft_ctx;
> +
> +enum debug_level {
> +	DEBUG_SCANNER		= 0x1,
> +	DEBUG_PARSER		= 0x2,
> +	DEBUG_EVALUATION	= 0x4,
> +	DEBUG_NETLINK		= 0x8,
> +	DEBUG_MNL		= 0x10,
> +	DEBUG_PROTO_CTX		= 0x20,
> +	DEBUG_SEGTREE		= 0x40,
> +};
> +
> +enum numeric_level {
> +	NUMERIC_NONE,
> +	NUMERIC_ADDR,
> +	NUMERIC_PORT,
> +	NUMERIC_ALL,
> +};

Just pushed out a patch to prepend NFT_ prefix.

> +/**
> + * Possible flags to pass to nft_ctx_new()
> + */
> +#define NFT_CTX_DEFAULT		0
> +
> +/**
> + * Exit codes returned by nft_run_cmd_from_*()
> + */
> +enum nftables_exit_codes {
> +	NFT_EXIT_SUCCESS	= 0,
> +	NFT_EXIT_FAILURE	= 1,
> +	NFT_EXIT_NOMEM		= 2,
> +	NFT_EXIT_NONL		= 3,
> +};

I think library is currently aborting in case of no-netlink and
no-memory, so these two error codes are not useful.

We would need to change codebase to propagate errors up to the
callers.

Regarding error code, I would go for -1 in case of error instead and 0
in case of success. If failure happens, then set errno with reason, so
we can get rid of these exit codes in a follow up patch?

  reply	other threads:[~2017-10-24 15:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-23 15:33 [nft PATCH v2 0/4] libnftables preparations Phil Sutter
2017-10-23 15:33 ` [nft PATCH v2 1/4] libnftables: Move library stuff out of main.c Phil Sutter
2017-10-24 15:48   ` Pablo Neira Ayuso [this message]
2017-10-24 16:42     ` Phil Sutter
2017-11-09 13:25     ` [nft PATCH] libnftables: Unexport enum nftables_exit_codes Phil Sutter
2017-11-09 19:27       ` Phil Sutter
2017-11-10 11:27         ` [nft PATCH v2] " Phil Sutter
2017-11-13 12:31           ` Pablo Neira Ayuso
2017-11-13 12:38             ` Phil Sutter
2017-11-13 14:08               ` [nft PATCH v3] " Phil Sutter
2017-11-16 13:33                 ` Pablo Neira Ayuso
2017-11-16 13:48                   ` Phil Sutter
2017-11-13 13:49             ` [nft PATCH v2] " Phil Sutter
2017-11-13 13:53               ` Pablo Neira Ayuso
2017-11-13 14:04                 ` Phil Sutter
2017-10-23 15:33 ` [nft PATCH v2 2/4] libnftables: Introduce nft_ctx_flush_cache() Phil Sutter
2017-10-24 15:52   ` Pablo Neira Ayuso
2017-10-24 17:40     ` Phil Sutter
2017-10-25  9:25       ` Pablo Neira Ayuso
2017-10-25 11:40         ` [nft PATCH] libnftables: Get rid of explicit cache flushes Phil Sutter
2017-10-26 18:15           ` Pablo Neira Ayuso
2017-10-23 15:33 ` [nft PATCH v2 3/4] cli: Use nft_run_cmd_from_buffer() Phil Sutter
2017-10-23 15:33 ` [nft PATCH v2 4/4] libnftables: Introduce getters and setters for everything Phil Sutter
2017-10-24 15:20 ` [nft PATCH v2 0/4] libnftables preparations Pablo Neira Ayuso
2017-10-24 16:29   ` Phil Sutter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171024154800.GA11705@salvia \
    --to=pablo@netfilter.org \
    --cc=eric@regit.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.