* [PATCH nft 0/3] scanner: improving include handling
@ 2020-02-05 12:29 Laurent Fasnacht
2020-02-05 12:29 ` [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure Laurent Fasnacht
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Laurent Fasnacht @ 2020-02-05 12:29 UTC (permalink / raw)
To: netfilter-devel
Hello,
I'd like to submit a small patch series to improve include behaviour. It fixes bug #1243 for example.
Let me know if you have any questions/remarks,
Best,
Laurent
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure
2020-02-05 12:29 [PATCH nft 0/3] scanner: improving include handling Laurent Fasnacht
@ 2020-02-05 12:29 ` Laurent Fasnacht
2020-02-07 16:50 ` Pablo Neira Ayuso
2020-02-05 12:30 ` [PATCH nft 2/3] scanner: correctly compute include depth Laurent Fasnacht
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Laurent Fasnacht @ 2020-02-05 12:29 UTC (permalink / raw)
To: netfilter-devel; +Cc: Laurent Fasnacht
This prevents a static allocation of file descriptors array, thus allows
more flexibility.
Signed-off-by: Laurent Fasnacht <fasnacht@protonmail.ch>
---
include/nftables.h | 3 ++-
src/scanner.l | 17 ++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/nftables.h b/include/nftables.h
index 90d33196..ca0fbcaf 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -122,7 +122,6 @@ struct nft_ctx {
void *scanner;
struct scope *top_scope;
void *json_root;
- FILE *f[MAX_INCLUDE_DEPTH];
};
enum nftables_exit_codes {
@@ -176,6 +175,7 @@ enum input_descriptor_types {
* struct input_descriptor
*
* @location: location, used for include statements
+ * @f: file descriptor
* @type: input descriptor type
* @name: name describing the input
* @union: buffer or file descriptor, depending on type
@@ -186,6 +186,7 @@ enum input_descriptor_types {
*/
struct input_descriptor {
struct list_head list;
+ FILE* f;
struct location location;
enum input_descriptor_types type;
const char *name;
diff --git a/src/scanner.l b/src/scanner.l
index 99ee8355..2016acd5 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -691,13 +691,13 @@ static void scanner_pop_buffer(yyscan_t scanner)
}
static void scanner_push_file(struct nft_ctx *nft, void *scanner,
- const char *filename, const struct location *loc)
+ FILE *f, const char *filename, const struct location *loc)
{
struct parser_state *state = yyget_extra(scanner);
struct input_descriptor *indesc;
YY_BUFFER_STATE b;
- b = yy_create_buffer(nft->f[state->indesc_idx], YY_BUF_SIZE, scanner);
+ b = yy_create_buffer(f, YY_BUF_SIZE, scanner);
yypush_buffer_state(b, scanner);
indesc = xzalloc(sizeof(struct input_descriptor));
@@ -706,6 +706,7 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
indesc->location = *loc;
indesc->type = INDESC_FILE;
indesc->name = xstrdup(filename);
+ indesc->f = f;
init_pos(indesc);
scanner_push_indesc(state, indesc);
@@ -731,8 +732,7 @@ static int include_file(struct nft_ctx *nft, void *scanner,
filename, strerror(errno));
goto err;
}
- nft->f[state->indesc_idx] = f;
- scanner_push_file(nft, scanner, filename, loc);
+ scanner_push_file(nft, scanner, f, filename, loc);
return 0;
err:
erec_queue(erec, state->msgs);
@@ -944,6 +944,10 @@ static void input_descriptor_list_destroy(struct parser_state *state)
struct input_descriptor *indesc, *next;
list_for_each_entry_safe(indesc, next, &state->indesc_list, list) {
+ if (indesc->f) {
+ fclose(indesc->f);
+ indesc->f = NULL;
+ }
list_del(&indesc->list);
input_descriptor_destroy(indesc);
}
@@ -955,11 +959,6 @@ void scanner_destroy(struct nft_ctx *nft)
do {
yypop_buffer_state(nft->scanner);
-
- if (nft->f[state->indesc_idx]) {
- fclose(nft->f[state->indesc_idx]);
- nft->f[state->indesc_idx] = NULL;
- }
} while (state->indesc_idx--);
input_descriptor_list_destroy(state);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nft 2/3] scanner: correctly compute include depth
2020-02-05 12:29 [PATCH nft 0/3] scanner: improving include handling Laurent Fasnacht
2020-02-05 12:29 ` [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure Laurent Fasnacht
@ 2020-02-05 12:30 ` Laurent Fasnacht
2020-02-07 17:00 ` Pablo Neira Ayuso
2020-02-05 12:30 ` [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list Laurent Fasnacht
2020-02-07 17:04 ` [PATCH nft 0/3] scanner: improving include handling Pablo Neira Ayuso
3 siblings, 1 reply; 8+ messages in thread
From: Laurent Fasnacht @ 2020-02-05 12:30 UTC (permalink / raw)
To: netfilter-devel; +Cc: Laurent Fasnacht
Inclusion depth was computed incorrectly for glob includes.
Signed-off-by: Laurent Fasnacht <fasnacht@protonmail.ch>
---
include/nftables.h | 2 ++
src/scanner.l | 20 ++++++++++++++------
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/nftables.h b/include/nftables.h
index ca0fbcaf..1d423738 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -176,6 +176,7 @@ enum input_descriptor_types {
*
* @location: location, used for include statements
* @f: file descriptor
+ * @depth: include depth of the descriptor
* @type: input descriptor type
* @name: name describing the input
* @union: buffer or file descriptor, depending on type
@@ -187,6 +188,7 @@ enum input_descriptor_types {
struct input_descriptor {
struct list_head list;
FILE* f;
+ unsigned int depth;
struct location location;
enum input_descriptor_types type;
const char *name;
diff --git a/src/scanner.l b/src/scanner.l
index 2016acd5..837bc476 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -691,7 +691,8 @@ static void scanner_pop_buffer(yyscan_t scanner)
}
static void scanner_push_file(struct nft_ctx *nft, void *scanner,
- FILE *f, const char *filename, const struct location *loc)
+ FILE *f, const char *filename, const struct location *loc,
+ const struct input_descriptor *parent_indesc)
{
struct parser_state *state = yyget_extra(scanner);
struct input_descriptor *indesc;
@@ -706,6 +707,11 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
indesc->location = *loc;
indesc->type = INDESC_FILE;
indesc->name = xstrdup(filename);
+ if (!parent_indesc) {
+ indesc->depth = 1;
+ } else {
+ indesc->depth = parent_indesc->depth + 1;
+ }
indesc->f = f;
init_pos(indesc);
@@ -714,13 +720,14 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
}
static int include_file(struct nft_ctx *nft, void *scanner,
- const char *filename, const struct location *loc)
+ const char *filename, const struct location *loc,
+ const struct input_descriptor *parent_indesc)
{
struct parser_state *state = yyget_extra(scanner);
struct error_record *erec;
FILE *f;
- if (state->indesc_idx == MAX_INCLUDE_DEPTH) {
+ if (parent_indesc && parent_indesc->depth == MAX_INCLUDE_DEPTH) {
erec = error(loc, "Include nested too deeply, max %u levels",
MAX_INCLUDE_DEPTH);
goto err;
@@ -732,7 +739,7 @@ static int include_file(struct nft_ctx *nft, void *scanner,
filename, strerror(errno));
goto err;
}
- scanner_push_file(nft, scanner, f, filename, loc);
+ scanner_push_file(nft, scanner, f, filename, loc, parent_indesc);
return 0;
err:
erec_queue(erec, state->msgs);
@@ -743,6 +750,7 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
const struct location *loc)
{
struct parser_state *state = yyget_extra(scanner);
+ struct input_descriptor *indesc = state->indesc;
struct error_record *erec = NULL;
bool wildcard = false;
glob_t glob_data;
@@ -803,7 +811,7 @@ static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
if (len == 0 || path[len - 1] == '/')
continue;
- ret = include_file(nft, scanner, path, loc);
+ ret = include_file(nft, scanner, path, loc, indesc);
if (ret != 0)
goto err;
}
@@ -840,7 +848,7 @@ err:
int scanner_read_file(struct nft_ctx *nft, const char *filename,
const struct location *loc)
{
- return include_file(nft, nft->scanner, filename, loc);
+ return include_file(nft, nft->scanner, filename, loc, NULL);
}
static bool search_in_include_path(const char *filename)
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list
2020-02-05 12:29 [PATCH nft 0/3] scanner: improving include handling Laurent Fasnacht
2020-02-05 12:29 ` [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure Laurent Fasnacht
2020-02-05 12:30 ` [PATCH nft 2/3] scanner: correctly compute include depth Laurent Fasnacht
@ 2020-02-05 12:30 ` Laurent Fasnacht
2020-02-07 17:03 ` Pablo Neira Ayuso
2020-02-07 17:04 ` [PATCH nft 0/3] scanner: improving include handling Pablo Neira Ayuso
3 siblings, 1 reply; 8+ messages in thread
From: Laurent Fasnacht @ 2020-02-05 12:30 UTC (permalink / raw)
To: netfilter-devel; +Cc: Laurent Fasnacht
parser_state.indescs (static array) and parser_state.indesc_list are in fact duplicating information.
This commit removes the static array and uses the list for everything.
Signed-off-by: Laurent Fasnacht <fasnacht@protonmail.ch>
---
include/parser.h | 2 --
src/scanner.l | 58 ++++++++++++++++++++----------------------------
2 files changed, 24 insertions(+), 36 deletions(-)
diff --git a/include/parser.h b/include/parser.h
index 949284d9..636d1c88 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -15,8 +15,6 @@
struct parser_state {
struct input_descriptor *indesc;
- struct input_descriptor *indescs[MAX_INCLUDE_DEPTH];
- unsigned int indesc_idx;
struct list_head indesc_list;
struct list_head *msgs;
diff --git a/src/scanner.l b/src/scanner.l
index 837bc476..4b06cb99 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -668,18 +668,23 @@ addrstring ({macaddr}|{ip4addr}|{ip6addr})
static void scanner_push_indesc(struct parser_state *state,
struct input_descriptor *indesc)
{
- state->indescs[state->indesc_idx] = indesc;
- state->indesc = state->indescs[state->indesc_idx++];
+ list_add_tail(&indesc->list, &state->indesc_list);
+ state->indesc = indesc;
}
static void scanner_pop_indesc(struct parser_state *state)
{
- state->indesc_idx--;
-
- if (state->indesc_idx > 0)
- state->indesc = state->indescs[state->indesc_idx - 1];
- else
+ struct input_descriptor *indesc;
+ indesc = state->indesc;
+ if (!list_empty(&state->indesc_list)) {
+ state->indesc = list_entry(state->indesc->list.prev, struct input_descriptor, list);
+ } else {
state->indesc = NULL;
+ }
+ if (indesc->f) {
+ fclose(indesc->f);
+ indesc->f = NULL;
+ }
}
static void scanner_pop_buffer(yyscan_t scanner)
@@ -716,7 +721,6 @@ static void scanner_push_file(struct nft_ctx *nft, void *scanner,
init_pos(indesc);
scanner_push_indesc(state, indesc);
- list_add_tail(&indesc->list, &state->indesc_list);
}
static int include_file(struct nft_ctx *nft, void *scanner,
@@ -915,15 +919,14 @@ void scanner_push_buffer(void *scanner, const struct input_descriptor *indesc,
{
struct parser_state *state = yyget_extra(scanner);
YY_BUFFER_STATE b;
+ struct input_descriptor *new_indesc;
- state->indesc = xzalloc(sizeof(struct input_descriptor));
- state->indescs[state->indesc_idx] = state->indesc;
- state->indesc_idx++;
+ new_indesc = xzalloc(sizeof(struct input_descriptor));
- memcpy(state->indesc, indesc, sizeof(*state->indesc));
- state->indesc->data = buffer;
- state->indesc->name = NULL;
- list_add_tail(&state->indesc->list, &state->indesc_list);
+ memcpy(new_indesc, indesc, sizeof(*new_indesc));
+ new_indesc->data = buffer;
+ new_indesc->name = NULL;
+ scanner_push_indesc(state, new_indesc);
b = yy_scan_string(buffer, scanner);
assert(b != NULL);
@@ -940,35 +943,22 @@ void *scanner_init(struct parser_state *state)
return scanner;
}
-static void input_descriptor_destroy(const struct input_descriptor *indesc)
-{
- if (indesc->name)
- xfree(indesc->name);
- xfree(indesc);
-}
-static void input_descriptor_list_destroy(struct parser_state *state)
+static void input_descriptor_list_destroy(yyscan_t *scanner)
{
+ struct parser_state *state = yyget_extra(scanner);
struct input_descriptor *indesc, *next;
list_for_each_entry_safe(indesc, next, &state->indesc_list, list) {
- if (indesc->f) {
- fclose(indesc->f);
- indesc->f = NULL;
- }
+ if (indesc->name)
+ xfree(indesc->name);
list_del(&indesc->list);
- input_descriptor_destroy(indesc);
+ xfree(indesc);
}
}
void scanner_destroy(struct nft_ctx *nft)
{
- struct parser_state *state = yyget_extra(nft->scanner);
-
- do {
- yypop_buffer_state(nft->scanner);
- } while (state->indesc_idx--);
-
- input_descriptor_list_destroy(state);
+ input_descriptor_list_destroy(nft->scanner);
yylex_destroy(nft->scanner);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure
2020-02-05 12:29 ` [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure Laurent Fasnacht
@ 2020-02-07 16:50 ` Pablo Neira Ayuso
0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-07 16:50 UTC (permalink / raw)
To: Laurent Fasnacht; +Cc: netfilter-devel
On Wed, Feb 05, 2020 at 12:29:55PM +0000, Laurent Fasnacht wrote:
> This prevents a static allocation of file descriptors array, thus allows
> more flexibility.
Applied, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft 2/3] scanner: correctly compute include depth
2020-02-05 12:30 ` [PATCH nft 2/3] scanner: correctly compute include depth Laurent Fasnacht
@ 2020-02-07 17:00 ` Pablo Neira Ayuso
0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-07 17:00 UTC (permalink / raw)
To: Laurent Fasnacht; +Cc: netfilter-devel
On Wed, Feb 05, 2020 at 12:30:13PM +0000, Laurent Fasnacht wrote:
> Inclusion depth was computed incorrectly for glob includes.
I get a segfault here when running your test after your patch 2/3 is
applied. I'm going to keep back this series, sorry.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list
2020-02-05 12:30 ` [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list Laurent Fasnacht
@ 2020-02-07 17:03 ` Pablo Neira Ayuso
0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-07 17:03 UTC (permalink / raw)
To: Laurent Fasnacht; +Cc: netfilter-devel
On Wed, Feb 05, 2020 at 12:30:24PM +0000, Laurent Fasnacht wrote:
[...]
> @@ -915,15 +919,14 @@ void scanner_push_buffer(void *scanner, const struct input_descriptor *indesc,
> {
> struct parser_state *state = yyget_extra(scanner);
> YY_BUFFER_STATE b;
> + struct input_descriptor *new_indesc;
>
> - state->indesc = xzalloc(sizeof(struct input_descriptor));
> - state->indescs[state->indesc_idx] = state->indesc;
> - state->indesc_idx++;
> + new_indesc = xzalloc(sizeof(struct input_descriptor));
I'd appreciate if you could split initially cleanups in separated
patches. Like adding this new variable.
> - memcpy(state->indesc, indesc, sizeof(*state->indesc));
> - state->indesc->data = buffer;
> - state->indesc->name = NULL;
> - list_add_tail(&state->indesc->list, &state->indesc_list);
> + memcpy(new_indesc, indesc, sizeof(*new_indesc));
> + new_indesc->data = buffer;
> + new_indesc->name = NULL;
> + scanner_push_indesc(state, new_indesc);
>
> b = yy_scan_string(buffer, scanner);
> assert(b != NULL);
> @@ -940,35 +943,22 @@ void *scanner_init(struct parser_state *state)
> return scanner;
> }
>
> -static void input_descriptor_destroy(const struct input_descriptor *indesc)
> -{
> - if (indesc->name)
> - xfree(indesc->name);
> - xfree(indesc);
> -}
Or removing this function, actually there is no need to check if
indesc->name != NULL, so just:
xfree(indesc->name);
xfree(indesc);
is probably fine while you are here.
That will make it easier for us to track changes.
Thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft 0/3] scanner: improving include handling
2020-02-05 12:29 [PATCH nft 0/3] scanner: improving include handling Laurent Fasnacht
` (2 preceding siblings ...)
2020-02-05 12:30 ` [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list Laurent Fasnacht
@ 2020-02-07 17:04 ` Pablo Neira Ayuso
3 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-07 17:04 UTC (permalink / raw)
To: Laurent Fasnacht; +Cc: netfilter-devel
On Wed, Feb 05, 2020 at 12:29:21PM +0000, Laurent Fasnacht wrote:
> Hello,
>
> I'd like to submit a small patch series to improve include
> behaviour. It fixes bug #1243 for example.
Thanks for addressing bugs. I'd suggest, let's start with making 1/3
and 2/3 fix the problem. Then following up with refinements is fine
too.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-02-07 17:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-05 12:29 [PATCH nft 0/3] scanner: improving include handling Laurent Fasnacht
2020-02-05 12:29 ` [PATCH nft 1/3] scanner: move the file descriptor to be in the input_descriptor structure Laurent Fasnacht
2020-02-07 16:50 ` Pablo Neira Ayuso
2020-02-05 12:30 ` [PATCH nft 2/3] scanner: correctly compute include depth Laurent Fasnacht
2020-02-07 17:00 ` Pablo Neira Ayuso
2020-02-05 12:30 ` [PATCH nft 3/3] scanner: remove indescs and indescs_idx attributes from the parser, and directly use indesc_list Laurent Fasnacht
2020-02-07 17:03 ` Pablo Neira Ayuso
2020-02-07 17:04 ` [PATCH nft 0/3] scanner: improving include handling Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).