[PATCH 0/2] ccli: Two small fixes for ccli

[PATCH 0/2] ccli: Two small fixes for ccli

[Ammar Faizi]

Hi Steve,

Alviro and I found two small issues in the ccli code.

The first one is a warning about uninitialized variable in the cache
code. The second one is an optimization issue found when clang warns
about unused return value of strlen() calls.

Fix them.

Ammar Faizi (2):
  ccli: cache: Fix -Wuninitialized warning
  ccli: commands: Fix optimized `strlen()` calls

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


base-commit: 418ad6a341a5ab6c21be666565d62a878aa3bd18
-- 
Ammar Faizi

[PATCH 1/2] ccli: cache: Fix -Wuninitialized warning

[Ammar Faizi]

The idx variable is uninitialized and incremented. Clang warns it:

  cache.c:214:28: warning: variable 'idx' is uninitialized when used here [-Wuninitialized]
    214 |         for (i = 0; i < cnt; i++, idx++) {
        |                                   ^~~
  cache.c:184:9: note: initialize the variable 'idx' to silence this warning
    184 |         int idx;
        |                ^
        |                 = 0

Remove the idx variable entierly, it's not used.

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>
---
 src/cache.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 010ddbd1e722..97a24066bf69 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -181,7 +181,6 @@ __hidden int cache_save_fd(struct ccli *ccli, const char *start_tag,
 	char *str;
 	char buf[64];
 	int ret;
-	int idx;
 	int i;
 
 	if (!ccli || !tag || !start_tag || !callback || fd < 0) {
@@ -211,7 +210,7 @@ __hidden int cache_save_fd(struct ccli *ccli, const char *start_tag,
 	if (ret < strlen(buf))
 		return -1;
 
-	for (i = 0; i < cnt; i++, idx++) {
+	for (i = 0; i < cnt; i++) {
 		if (callback(ccli, fd, i, cnt, data) < 0)
 			break;
 	}
-- 
Ammar Faizi

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

[Ammar Faizi]

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

Re: [PATCH 0/2] ccli: Two small fixes for ccli

[Steven Rostedt]

On Thu, 21 Aug 2025 04:14:32 +0700
Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:

> Hi Steve,

Hi Ammar!

> 
> Alviro and I found two small issues in the ccli code.

There's a famous Linux kernel developer named Al Viro, and when I saw the
name Alviro, I was confused and was wondering why Al was working with you.
But then I saw the Cc list ;-)

> 
> The first one is a warning about uninitialized variable in the cache
> code. The second one is an optimization issue found when clang warns
> about unused return value of strlen() calls.
> 
> Fix them.
> 
> Ammar Faizi (2):
>   ccli: cache: Fix -Wuninitialized warning
>   ccli: commands: Fix optimized `strlen()` calls

Thanks for the updates!

FYI, I'm relicensing libccli under MIT from LGPL. Are you OK with that?

Cheers,

-- Steve


> 
>  src/cache.c      | 3 +--
>  src/ccli-local.h | 1 +
>  src/commands.c   | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> base-commit: 418ad6a341a5ab6c21be666565d62a878aa3bd18

Re: [PATCH 0/2] ccli: Two small fixes for ccli

[Ammar Faizi]

On Wed, Aug 20, 2025 at 08:12:36PM -0400, Steven Rostedt wrote:
> Thanks for the updates!
> 
> FYI, I'm relicensing libccli under MIT from LGPL. Are you OK with that?

Yes, that is fine for me.

-- 
Ammar Faizi

Re: [PATCH 0/2] ccli: Two small fixes for ccli

[Steven Rostedt]

On Thu, 21 Aug 2025 09:15:55 +0900
Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:

> On Wed, Aug 20, 2025 at 08:12:36PM -0400, Steven Rostedt wrote:
> > Thanks for the updates!
> > 
> > FYI, I'm relicensing libccli under MIT from LGPL. Are you OK with that?  
> 
> Yes, that is fine for me.
> 

Ah, sorry, you already acked it. ;-)

-- Steve

Re: [PATCH 0/2] ccli: Two small fixes for ccli

[Steven Rostedt]

On Wed, 20 Aug 2025 20:18:51 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 21 Aug 2025 09:15:55 +0900
> Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
> 
> > On Wed, Aug 20, 2025 at 08:12:36PM -0400, Steven Rostedt wrote:  
> > > Thanks for the updates!
> > > 
> > > FYI, I'm relicensing libccli under MIT from LGPL. Are you OK with that?    
> > 
> > Yes, that is fine for me.
> >   
> 
> Ah, sorry, you already acked it. ;-)

And I just merged all the changes to the main branch.

It's still not ready for 1.2 release (hence the "dev"), but it's getting there.

Unfortunately, I need to focus on Linux kernel changes for a while, so this
is going back on the back burner :-p

-- Steve