[PATCH 2/2] ccli: commands: Fix optimized `strlen()` calls

[Ammar Faizi <ammarfaizi2@gnuweeb.org>]

When compiling with clang, the following warning appears:

  commands.c:291:6: warning: variable 'l' set but not used [-Wunused-but-set-variable]
    291 |         int l = 0;
        |             ^

Closer inspection shows that `l` is only used to accumulate the return
values of `strlen()` calls, but its value is never read. It turns out
that it indicates a deeper issue than a simple unused variable.

The purpose of the `strlen()` calls in this code is to ensure all
strings passed to `test_table()` are touched. However, `strlen()` is
declared with `__attribute_pure__`.

A pure function always produces the same result for the same input and
has no side effects.

If the result of `strlen()` is not used, the compiler is allowed to
remove the call entirely, which means the strings are never actually
touched.

For example, compiling with `-O3`:

        #include <string.h>
        void touch_string(char *s)
        {
                strlen(s);
        }

produces only:

        touch_string:
            retq

Even at `-O0`, gcc still omits the `strlen()` call:

        touch_string:
            pushq   %rbp
            movq    %rsp,%rbp
            movq    %rdi,-0x8(%rbp)
            nop
            popq    %rbp
            retq

Introduce a new macro, OPTIMIZER_HIDE_VAR(), to hide the variable from
the compiler's optimizer, keeping the strlen() calls intact. It also
cleans the clang warning.

Co-authored-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
Signed-off-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Giving more context for src/commands.c hunk for easier review.

 src/ccli-local.h | 1 +
 src/commands.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/ccli-local.h b/src/ccli-local.h
index 31d16de73095..d7fd8e71e482 100644
--- a/src/ccli-local.h
+++ b/src/ccli-local.h
@@ -15,6 +15,7 @@
 #define __hidden __attribute__((visibility ("hidden")))
 
 #define ISSPACE(c) isspace((unsigned char)(c))
+#define OPTIMIZER_HIDE_VAR(X) __asm__ volatile ("": "+r" (X))
 
 struct line_buf {
 	char *line;
diff --git a/src/commands.c b/src/commands.c
--- a/src/commands.c
+++ b/src/commands.c
@@ -285,49 +285,50 @@ int ccli_execute(struct ccli *ccli, const char *line_str, bool hist)
 
 static void test_table(const void *data)
 {
 	const struct ccli_command_table *table = data;
 	const char *name = table->name;
 	int len = strlen(test_name);
 	int l = 0;
 	int i;
 
 	/* The root of the table does not need a name */
 	if (!name)
 		name = "root";
 
 	if (len)
 		strncat(test_name, "->", BUFSIZ - 1);
 	strncat(test_name, name, BUFSIZ - 1);
 
 	snprintf(test_message, BUFSIZ - 1,
 		 "Command table missing name or 'subcommands' NULL terminator");
 
 	/* Touch all the names first */
 	for (i = 0; table->subcommands[i]; i++)
 		l += strlen(table->subcommands[i]->name);
 
 	snprintf(test_message, BUFSIZ - 1,
 		 "Command table missing 'options' NULL terminator");
 
 	if (table->options) {
 		int clen;
 
 		/* Test the options too */
 
 		clen = strlen(test_name);
 		if (clen)
 			strncat(test_name, "->[options]", BUFSIZ - 1);
 
 		for (i = 0; table->options->options[i]; i++)
 			l += strlen(table->options->options[i]->name);
 
 		test_name[clen] = '\0';
 	}
 
 	/* Now recurse */
 	for (i = 0; table->subcommands[i]; i++)
 		test_table(table->subcommands[i]);
 
 	test_name[len] = '\0';
+	OPTIMIZER_HIDE_VAR(l);
 }
 

-- 
Ammar Faizi