From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Steven Barth <cyrus@openwrt.org>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [nft PATCH] build: allow disabling libreadline-support
Date: Thu, 9 Oct 2014 18:33:10 +0200 [thread overview]
Message-ID: <20141009163310.GA10706@salvia> (raw)
In-Reply-To: <1412866961.4287.1.camel@openwrt.org>
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
On Thu, Oct 09, 2014 at 05:02:41PM +0200, Steven Barth wrote:
> > Please, add the:
> >
> > #ifdef HAVE_LIBREADLINE
> >
> > in cli_init() and cli_exit() in cli.c. It would be good if the cli_init()
> > returns a negative value, so you spot the error message below.
>
> The problem I see here that this would basically mean enclosing most of
> cli.c in #ifdef HAVE_LIBREADLINE to avoid depending on readline-headers
> and having a rather awkward stub of cli_init in the no-readline case.
>
> My main point of excluding cli.c in the Makefile from the start is to
> avoid this madness.
I see, then I'd suggest something like the change (applies on top of
your patch).
Would you merge this to your patch, including the --without-cli option
and resend? Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 1229 bytes --]
diff --git a/include/nftables.h b/include/nftables.h
index 3394e32..c3d3dbf 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -31,7 +31,14 @@ extern unsigned int debug_level;
extern const char *include_paths[INCLUDE_PATHS_MAX];
struct parser_state;
+#ifdef HAVE_LIBREADLINE
extern int cli_init(struct parser_state *state);
+#else
+static inline int cli_init(struct parser_state *state)
+{
+ return -1;
+}
+#endif
extern void cli_exit(void);
extern void cli_display(const char *fmt, va_list ap) __fmtstring(1, 0);
diff --git a/src/main.c b/src/main.c
index de2efe6..8dc0768 100644
--- a/src/main.c
+++ b/src/main.c
@@ -335,14 +335,12 @@ int main(int argc, char * const *argv)
if (scanner_read_file(scanner, filename, &internal_location) < 0)
goto out;
} else if (interactive) {
-#ifdef HAVE_LIBREADLINE
- cli_init(&state);
+ if (cli_init(&state) < 0) {
+ fprintf(stderr, "%s: CLI not supported in this build\n",
+ argv[0]);
+ exit(NFT_EXIT_FAILURE);
+ }
return 0;
-#else
- fprintf(stderr, "%s: interactive CLI not supported in this build\n",
- argv[0]);
- exit(NFT_EXIT_FAILURE);
-#endif
} else {
fprintf(stderr, "%s: no command specified\n", argv[0]);
exit(NFT_EXIT_FAILURE);
next prev parent reply other threads:[~2014-10-09 16:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-01 21:59 [nft PATCH] build: allow disabling libreadline-support Steven Barth
2014-10-09 12:26 ` Pablo Neira Ayuso
2014-10-09 15:02 ` Steven Barth
2014-10-09 16:33 ` Pablo Neira Ayuso [this message]
2014-10-09 20:48 ` [nft PATCHv2] " Steven Barth
2014-10-10 10:23 ` Pablo Neira Ayuso
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=20141009163310.GA10706@salvia \
--to=pablo@netfilter.org \
--cc=cyrus@openwrt.org \
--cc=netfilter-devel@vger.kernel.org \
/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.