Re: [LTP] [PATCH v2 4/7] docparse: Implement ARRAY_SIZE()

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Adds a special handlingn for ARRAY_SIZE() macro.
>
> If we stumble upon ARRAY_SIZE() in the tst_test structure we try to
> lookup the array and count its members.
>
> Proper parsing of .test_variants also requires that we add -I switch to
> the docparse to be able to specify include paths on a command line since
> some variants are stuck in top level include while others are in
> testcases/kernel/syscalls/utils/.
>
> + tests
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>

Again I don't see anything that would presently prevent parsing. But see
note below

Reviewed-by: rpalethorpe@suse.com

> ---
>  docparse/docparse.c                | 221 ++++++++++++++++++++++++++---
>  docparse/parse.sh                  |   2 +-
>  docparse/tests/array_size01.c      |   5 +
>  docparse/tests/array_size01.c.json |   4 +
>  docparse/tests/array_size02.c      |   9 ++
>  docparse/tests/array_size02.c.json |   4 +
>  docparse/tests/array_size03.c      |  10 ++
>  docparse/tests/array_size03.c.json |   4 +
>  docparse/tests/array_size04.c      |   5 +
>  docparse/tests/array_size04.c.json |   4 +
>  docparse/tests/include.h           |   6 +
>  11 files changed, 255 insertions(+), 19 deletions(-)
>  create mode 100644 docparse/tests/array_size01.c
>  create mode 100644 docparse/tests/array_size01.c.json
>  create mode 100644 docparse/tests/array_size02.c
>  create mode 100644 docparse/tests/array_size02.c.json
>  create mode 100644 docparse/tests/array_size03.c
>  create mode 100644 docparse/tests/array_size03.c.json
>  create mode 100644 docparse/tests/array_size04.c
>  create mode 100644 docparse/tests/array_size04.c.json
>
> diff --git a/docparse/docparse.c b/docparse/docparse.c
> index 64f9d08d9..9133accf5 100644
> --- a/docparse/docparse.c
> +++ b/docparse/docparse.c
> @@ -15,7 +15,11 @@
>  
>  #include "data_storage.h"
>  
> +#define INCLUDE_PATH_MAX 5
> +
>  static int verbose;
> +static char *cmdline_includepath[INCLUDE_PATH_MAX];
> +static unsigned int cmdline_includepaths;
>  static char *includepath;
>  
>  #define WARN(str) fprintf(stderr, "WARNING: " str "\n")
> @@ -132,13 +136,14 @@ static void maybe_comment(FILE *f, struct data_node *doc)
>  	}
>  }
>  
> -static char *next_token(FILE *f, struct data_node *doc)
> +static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *doc)
>  {
>  	size_t i = 0;
> -	static char buf[4096];
>  	int c;
>  	int in_str = 0;
>  
> +	buf_len--;
> +
>  	for (;;) {
>  		c = fgetc(f);
>  
> @@ -151,7 +156,8 @@ static char *next_token(FILE *f, struct data_node *doc)
>  					goto exit;
>  			}
>  
> -			buf[i++] = c;
> +			if (i < buf_len)
> +				buf[i++] = c;
>  			continue;
>  		}
>  
> @@ -171,7 +177,8 @@ static char *next_token(FILE *f, struct data_node *doc)
>  				goto exit;
>  			}
>  
> -			buf[i++] = c;
> +			if (i < buf_len)
> +				buf[i++] = c;
>  			goto exit;
>  		case '0' ... '9':
>  		case 'a' ... 'z':
> @@ -204,11 +211,33 @@ exit:
>  	return buf;
>  }
>  
> -static FILE *open_include(const char *includepath, FILE *f)
> +static char *next_token(FILE *f, struct data_node *doc)
> +{
> +	static char buf[4096];
> +
> +	return next_token2(f, buf, sizeof(buf), doc);
> +}
> +
> +static FILE *open_file(const char *dir, const char *fname)
>  {
> -	char buf[256];
> +	FILE *f;
>  	char *path;
> +
> +	if (asprintf(&path, "%s/%s", dir, fname) < 0)
> +		return NULL;
> +
> +	f = fopen(path, "r");
> +
> +	free(path);
> +
> +	return f;
> +}
> +
> +static FILE *open_include(FILE *f)
> +{
> +	char buf[256], *fname;
>  	FILE *inc;
> +	unsigned int i;
>  
>  	if (!fscanf(f, "%s\n", buf))
>  		return NULL;
> @@ -216,24 +245,36 @@ static FILE *open_include(const char *includepath, FILE *f)
>  	if (buf[0] != '"')
>  		return NULL;
>  
> -	char *filename = buf + 1;
> +	fname = buf + 1;
>  
>  	if (!buf[0])
>  		return NULL;
>  
> -	filename[strlen(filename)-1] = 0;
> +	fname[strlen(fname)-1] = 0;
>  
> -	if (asprintf(&path, "%s/%s", includepath, filename) < 0)
> -		return NULL;
> +	inc = open_file(includepath, fname);
> +	if (inc) {
> +		if (verbose)
> +			fprintf(stderr, "INCLUDE %s/%s\n", includepath, fname);
>  
> -	inc = fopen(path, "r");
> +		return inc;
> +	}
>  
> -	if (inc && verbose)
> -		fprintf(stderr, "INCLUDE %s\n", path);
> +	for (i = 0; i < cmdline_includepaths; i++) {
> +		inc = open_file(cmdline_includepath[i], fname);
>  
> -	free(path);
> +		if (!inc)
> +			continue;
>  
> -	return inc;
> +		if (verbose) {
> +			fprintf(stderr, "INCLUDE %s/%s\n",
> +				cmdline_includepath[i], fname);
> +		}
> +
> +		return inc;
> +	}
> +
> +	return NULL;
>  }
>  
>  static void close_include(FILE *inc)
> @@ -300,6 +341,136 @@ static void try_apply_macro(char **res)
>  	*res = ret->data;
>  }
>  
> +static int parse_get_array_len(FILE *f)
> +{
> +	const char *token;
> +	int cnt = 0, depth = 0, prev_comma = 0;
> +
> +	if (!(token = next_token(f, NULL)))
> +		return 0;
> +
> +	if (strcmp(token, "{"))
> +		return 0;
> +
> +	for (;;) {
> +		if (!(token = next_token(f, NULL)))
> +			return 0;
> +
> +		if (!strcmp(token, "{"))
> +			depth++;
> +
> +		if (!strcmp(token, "}"))
> +			depth--;
> +		else
> +			prev_comma = 0;
> +
> +		if (!strcmp(token, ",") && !depth) {
> +			prev_comma = 1;
> +			cnt++;
> +		}
> +
> +		if (depth < 0)
> +			return cnt + !prev_comma;
> +	}
> +}
> +
> +static void look_for_array_size(FILE *f, const char *arr_id, struct data_node **res)
> +{
> +	const char *token;
> +	char buf[2][2048] = {};
> +	int cur_buf = 0;
> +	int prev_buf = 1;
> +
> +	for (;;) {
> +		if (!(token = next_token2(f, buf[cur_buf], 2048, NULL)))
> +			break;
> +
> +		if (!strcmp(token, "=") && !strcmp(buf[prev_buf], arr_id)) {
> +			int arr_len = parse_get_array_len(f);
> +
> +			if (verbose)
> +				fprintf(stderr, "ARRAY %s LENGTH = %i\n", arr_id, arr_len);
> +
> +			*res = data_node_int(arr_len);
> +
> +			break;
> +		}
> +
> +		if (strcmp(buf[cur_buf], "]") && strcmp(buf[cur_buf], "[")) {

So this seems to mean we would silently ignore a variants array which
was declared with an explicit size. A quick grep doesn't turn up any
instances of that. However this would be an unexpected result. People
may intuitively think adding an explicity size would make parsing
easier.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp