Hello all,

It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.

For example:
  keyword '' arg_2

In that case the content of the args field will be like this:
  args[0]:                  'keyword'
  args[1]:                  NULL pointer
  args[2]:                  'arg_2'
  args[3 .. MAX_LINE_ARGS): NULL pointers

The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.

All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix.  This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.

Best regards,

--
Zaga    <[email protected]>

What can change the nature of a man?
>From 0c0c0c77fe5b1a71ea639b5693eab85a917e9df0 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac <[email protected]>
Date: Wed, 14 Apr 2021 11:44:58 +0200
Subject: [PATCH 1/2] MINOR: opentracing: correct calculation of the number of
 arguments in the args[]

It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.

For example:
  keyword '' arg_2

In that case the content of the args field will be like this:
  args[0]:                  'keyword'
  args[1]:                  NULL pointer
  args[2]:                  'arg_2'
  args[3 .. MAX_LINE_ARGS): NULL pointers

The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.

All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix.  This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.
---
 addons/ot/src/parser.c | 17 ++++++++--------
 addons/ot/src/util.c   | 44 +++++++++++++++++++++++++++++++-----------
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index c7522e942..5dec8629d 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -189,7 +189,7 @@ static const char *flt_ot_parse_invalid_char(const char *name, int type)
  */
 static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, const void *id, const struct flt_ot_parse_data *parse_data, size_t parse_data_size, const struct flt_ot_parse_data **pdata, char **err)
 {
-	int i, retval = ERR_NONE;
+	int i, argc, retval = ERR_NONE;
 
 	FLT_OT_FUNC("\"%s\", %d, %p, %p, %p, %zu, %p:%p, %p:%p", file, linenum, args, id, parse_data, parse_data_size, FLT_OT_DPTR_ARGS(pdata), FLT_OT_DPTR_ARGS(err));
 
@@ -197,12 +197,15 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
 
 	*pdata = NULL;
 
+	/* First check here if args[0] is the correct keyword. */
 	for (i = 0; (*pdata == NULL) && (i < parse_data_size); i++)
 		if (strcmp(parse_data[i].name, args[0]) == 0)
 			*pdata = parse_data + i;
 
 	if (*pdata == NULL)
 		FLT_OT_PARSE_ERR(err, "'%s' : unknown keyword", args[0]);
+	else
+		argc = flt_ot_args_count(args);
 
 	if ((retval & ERR_CODE) || (id == NULL))
 		/* Do nothing. */;
@@ -214,20 +217,16 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
 	 * line than is required.
 	 */
 	if (!(retval & ERR_CODE))
-		for (i = 1; i < (*pdata)->args_min; i++)
-			if (!FLT_OT_ARG_ISVALID(i))
-				FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
+		if (argc < (*pdata)->args_min)
+			FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
 
 	/*
 	 * Checking that more arguments are specified in the configuration
 	 * line than the maximum allowed.
 	 */
-	if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0)) {
-		for ( ; (i <= (*pdata)->args_max) && FLT_OT_ARG_ISVALID(i); i++);
-
-		if (i > (*pdata)->args_max)
+	if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0))
+		if (argc > (*pdata)->args_max)
 			FLT_OT_PARSE_ERR(err, "'%s' : too many arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
-	}
 
 	/* Checking that the first argument has only allowed characters. */
 	if (!(retval & ERR_CODE) && ((*pdata)->check_name > 0)) {
diff --git a/addons/ot/src/util.c b/addons/ot/src/util.c
index 3adc5a300..f6812bcc2 100644
--- a/addons/ot/src/util.c
+++ b/addons/ot/src/util.c
@@ -37,13 +37,13 @@
  */
 void flt_ot_args_dump(char **args)
 {
-	int i, n;
+	int i, argc;
 
-	for (n = 1; FLT_OT_ARG_ISVALID(n); n++);
+	argc = flt_ot_args_count(args);
 
-	(void)fprintf(stderr, FLT_OT_DBG_FMT("%.*sargs[%d]: { '%s' "), dbg_indent_level, FLT_OT_DBG_INDENT, n, args[0]);
+	(void)fprintf(stderr, FLT_OT_DBG_FMT("%.*sargs[%d]: { '%s' "), dbg_indent_level, FLT_OT_DBG_INDENT, argc, args[0]);
 
-	for (i = 1; FLT_OT_ARG_ISVALID(i); i++)
+	for (i = 1; i < argc; i++)
 		(void)fprintf(stderr, "'%s' ", args[i]);
 
 	(void)fprintf(stderr, "}\n");
@@ -365,10 +365,30 @@ ssize_t flt_ot_chunk_add(struct buffer *chk, const void *src, size_t n, char **e
  */
 int flt_ot_args_count(char **args)
 {
-	int retval = 0;
-
-	if (args != NULL)
-		for ( ; FLT_OT_ARG_ISVALID(retval); retval++);
+	int i, retval = 0;
+
+	if (args == NULL)
+		return retval;
+
+	/*
+	 * It is possible that some arguments within the configuration line
+	 * are not specified; that is, they are set to a blank string.
+	 *
+	 * For example:
+	 *   keyword '' arg_2
+	 *
+	 * In that case the content of the args field will be like this:
+	 *   args[0]:                  'keyword'
+	 *   args[1]:                  NULL pointer
+	 *   args[2]:                  'arg_2'
+	 *   args[3 .. MAX_LINE_ARGS): NULL pointers
+	 *
+	 * The total number of arguments is the index of the last argument
+	 * (increased by 1) that is not a NULL pointer.
+	 */
+	for (i = 0; i < MAX_LINE_ARGS; i++)
+		if (FLT_OT_ARG_ISVALID(i))
+			retval = i + 1;
 
 	return retval;
 }
@@ -391,13 +411,15 @@ int flt_ot_args_count(char **args)
  */
 void flt_ot_args_to_str(char **args, int idx, char **str)
 {
-	int i;
+	int i, argc;
 
 	if ((args == NULL) || (*args == NULL))
 		return;
 
-	for (i = idx; FLT_OT_ARG_ISVALID(i); i++)
-		(void)memprintf(str, "%s%s%s", (*str == NULL) ? "" : *str, (i == idx) ? "" : " ", args[i]);
+	argc = flt_ot_args_count(args);
+
+	for (i = idx; i < argc; i++)
+		(void)memprintf(str, "%s%s%s", (*str == NULL) ? "" : *str, (i == idx) ? "" : " ", (args[i] == NULL) ? "" : args[i]);
 }
 
 
-- 
2.30.1

Reply via email to