[PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()

Thorsten Blum posted 1 patch 1 month, 3 weeks ago
scripts/kconfig/nconf.gui.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()
Posted by Thorsten Blum 1 month, 3 weeks ago
Use 'min(len, x)' as the index instead of just 'len' to NUL-terminate
the copied 'line' string at the correct position.

Add a newline after the local variable declarations to silence a
checkpatch warning.

Cc: stable@vger.kernel.org
Fixes: 692d97c380c6 ("kconfig: new configuration interface (nconfig)")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 scripts/kconfig/nconf.gui.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 7206437e784a..ec021ebd2c52 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -175,8 +175,9 @@ void fill_window(WINDOW *win, const char *text)
 	for (i = 0; i < total_lines; i++) {
 		char tmp[x+10];
 		const char *line = get_line(text, i);
-		int len = get_line_length(line);
-		strncpy(tmp, line, min(len, x));
+		int len = min(get_line_length(line), x);
+
+		strncpy(tmp, line, len);
 		tmp[len] = '\0';
 		mvwprintw(win, i, 0, "%s", tmp);
 	}
-- 
2.50.1
Re: [PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()
Posted by Franco Martelli 1 month, 3 weeks ago
On 10/08/25 at 16:22, Thorsten Blum wrote:
> Use 'min(len, x)' as the index instead of just 'len' to NUL-terminate
> the copied 'line' string at the correct position.
> 
> Add a newline after the local variable declarations to silence a
> checkpatch warning.
> 
> Cc: stable@vger.kernel.org
> Fixes: 692d97c380c6 ("kconfig: new configuration interface (nconfig)")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>   scripts/kconfig/nconf.gui.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index 7206437e784a..ec021ebd2c52 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -175,8 +175,9 @@ void fill_window(WINDOW *win, const char *text)
>   	for (i = 0; i < total_lines; i++) {
>   		char tmp[x+10];
>   		const char *line = get_line(text, i);
> -		int len = get_line_length(line);
> -		strncpy(tmp, line, min(len, x));
> +		int len = min(get_line_length(line), x);
> +
> +		strncpy(tmp, line, len);
>   		tmp[len] = '\0';
>   		mvwprintw(win, i, 0, "%s", tmp);
>   	}

Is there a rationale behind the choice to avoid to use snprintf()
in these circumstance?
Preferring snprintf() you will not have to take care to
compute the position of the NULL terminating character of the
string, it's done automatically by this function.

Best regards,
-- 
Franco Martelli
Re: [PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()
Posted by Thorsten Blum 1 month, 3 weeks ago
On 11. Aug 2025, at 14:51, Franco Martelli wrote:
> Is there a rationale behind the choice to avoid to use snprintf()
> in these circumstance?
> Preferring snprintf() you will not have to take care to
> compute the position of the NULL terminating character of the
> string, it's done automatically by this function.

I looked into it a bit more and I think we need neither strncpy() nor
snprintf() (and no temporary buffer) because this should be sufficient:

	mvwprintw(win, i, 0, "%.*s", len, line);

Unless I'm missing something, I'm happy to send a v2.

Thanks,
Thorsten
Re: [PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()
Posted by Thorsten Blum 1 month, 3 weeks ago
On 11. Aug 2025, at 17:08, Thorsten Blum wrote:
> On 11. Aug 2025, at 14:51, Franco Martelli wrote:
>> Is there a rationale behind the choice to avoid to use snprintf()
>> in these circumstance?
>> Preferring snprintf() you will not have to take care to
>> compute the position of the NULL terminating character of the
>> string, it's done automatically by this function.
> 
> I looked into it a bit more and I think we need neither strncpy() nor
> snprintf() (and no temporary buffer) because this should be sufficient:
> 
> 	mvwprintw(win, i, 0, "%.*s", len, line);
> 
> Unless I'm missing something, I'm happy to send a v2.

I submitted this as a new patch instead of a v2:

https://lore.kernel.org/lkml/20250811161650.37428-2-thorsten.blum@linux.dev/

Thanks,
Thorsten
Re: [PATCH] kconfig: nconf: NUL-terminate 'line' correctly in fill_window()
Posted by Randy Dunlap 1 month, 3 weeks ago

On 8/10/25 7:22 AM, Thorsten Blum wrote:
> Use 'min(len, x)' as the index instead of just 'len' to NUL-terminate
> the copied 'line' string at the correct position.
> 
> Add a newline after the local variable declarations to silence a
> checkpatch warning.
> 
> Cc: stable@vger.kernel.org
> Fixes: 692d97c380c6 ("kconfig: new configuration interface (nconfig)")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

LGTM. Thanks.

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

> ---
>  scripts/kconfig/nconf.gui.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index 7206437e784a..ec021ebd2c52 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -175,8 +175,9 @@ void fill_window(WINDOW *win, const char *text)
>  	for (i = 0; i < total_lines; i++) {
>  		char tmp[x+10];
>  		const char *line = get_line(text, i);
> -		int len = get_line_length(line);
> -		strncpy(tmp, line, min(len, x));
> +		int len = min(get_line_length(line), x);
> +
> +		strncpy(tmp, line, len);
>  		tmp[len] = '\0';
>  		mvwprintw(win, i, 0, "%s", tmp);
>  	}

-- 
~Randy