Change in coreboot[master]: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str`
Jude Rich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37939 )
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
utils/kconfig/nconf.c: Fix truncation warning in `item_add_str`
While appending a string in `item_add_str`, there is a warning about truncating the string to 256 bytes due to unnecessary buffering. By removing the buffering and writing directly to the string, the warning is vanquished.
Signed-off-by: Jude A Rich juder11@gmail.com Change-Id: Idb2dfad7e401954f4bb83d0409ab71dcd7277f47 --- M util/kconfig/nconf.c 1 file changed, 18 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/37939/1
diff --git a/util/kconfig/nconf.c b/util/kconfig/nconf.c index 905dcd1..4181c41 100644 --- a/util/kconfig/nconf.c +++ b/util/kconfig/nconf.c @@ -561,32 +561,33 @@ curses_menu_items[items_num] = NULL; }
-/* very hackish. adds a string to the last item added */ +/* add a string to the last item added */ static void item_add_str(const char *fmt, ...) { va_list ap; - int index = items_num-1; - char new_str[256]; - char tmp_str[256]; + char *append_ptr; /* pointer to the free space in the target string */ + int index = items_num-1; /* index of item to append string */ + int kmi_str_len; /* length of initial string */ + int str_space; /* length of unused space in string */
+ /* return if there's no item to work with */ if (index < 0) return;
- va_start(ap, fmt); - vsnprintf(new_str, sizeof(new_str), fmt, ap); - va_end(ap); - snprintf(tmp_str, sizeof(tmp_str), "%s%s", - k_menu_items[index].str, new_str); - strncpy(k_menu_items[index].str, - tmp_str, - sizeof(k_menu_items[index].str)); + kmi_str_len = strlen(k_menu_items[index].str); + str_space = sizeof(k_menu_items[index].str) - kmi_str_len; + append_ptr = k_menu_items[index].str + kmi_str_len;
+ /* append the string */ + va_start(ap, fmt); + vsnprintf(append_ptr, str_space, fmt, ap); + va_end(ap); + + /* set menu item to new item string */ free_item(curses_menu_items[index]); - curses_menu_items[index] = new_item( - k_menu_items[index].str, - k_menu_items[index].str); - set_item_userptr(curses_menu_items[index], - &k_menu_items[index]); + curses_menu_items[index] = new_item(k_menu_items[index].str, + k_menu_items[index].str); + set_item_userptr(curses_menu_items[index], &k_menu_items[index]); }
/* get the tag of the currently selected item */
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37939
to look at the new patch set (#2).
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
utils/kconfig/nconf.c: Fix truncation warning in `item_add_str`
While appending a string in `item_add_str`, there is a warning about truncating the string to 256 bytes due to unnecessary buffering. By removing the buffering and writing directly to the string, the warning is vanquished.
Signed-off-by: Jude A Rich juder11@gmail.com Change-Id: Idb2dfad7e401954f4bb83d0409ab71dcd7277f47 --- M util/kconfig/nconf.c 1 file changed, 18 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/37939/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37939 )
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
Patch Set 2:
(3 comments)
Hello Jude, thanks for your patch. Do you intend to upstream it to Linux? Also, I'm curious, is this fixing an actual issues or is it only about cosmetics?
https://review.coreboot.org/c/coreboot/+/37939/2/util/kconfig/nconf.c File util/kconfig/nconf.c:
https://review.coreboot.org/c/coreboot/+/37939/2/util/kconfig/nconf.c@570 PS2, Line 570: int kmi_str_len; /* length of initial string */ : int str_space; /* length of unused space in string */ These could be `size_t`.
https://review.coreboot.org/c/coreboot/+/37939/2/util/kconfig/nconf.c@577 PS2, Line 577: kmi_str_len = strlen(k_menu_items[index].str); Using strnlen() would avoid potential overflows if .str isn't properly terminated.
https://review.coreboot.org/c/coreboot/+/37939/2/util/kconfig/nconf.c@589 PS2, Line 589: k_menu_items[index].str); This is oddly indented (and I don't see how changing this makes anything better).
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37939
to look at the new patch set (#3).
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
utils/kconfig/nconf.c: Fix truncation warning in `item_add_str`
While appending a string in `item_add_str`, there is a warning about truncating the string to 256 bytes due to unnecessary buffering. By removing the buffering and writing directly to the string, the warning is vanquished.
Signed-off-by: Jude A Rich juder11@gmail.com Change-Id: Idb2dfad7e401954f4bb83d0409ab71dcd7277f47 --- M util/kconfig/nconf.c 1 file changed, 17 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/37939/3
Jude Rich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37939 )
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
Patch Set 3:
Patch Set 2:
(3 comments)
Hello Jude, thanks for your patch. Do you intend to upstream it to Linux? Also, I'm curious, is this fixing an actual issues or is it only about cosmetics?
Hello, Niko. Thank you for the review. Your points were all valid, so I made the changes. I don't believe there was a bug here, it's mainly about cleaner code.
As for upstreaming to Linux, I'll have to do that. It hadn't occurred to me that they were the same.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37939 )
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
Patch Set 3:
Patrick, Martin, what do you think, should we pull this in or wait for upstream?
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37939?usp=email )
Change subject: utils/kconfig/nconf.c: Fix truncation warning in `item_add_str` ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Jude Rich (Code Review) -
Martin L Roth (Code Review)
-
Nico Huber (Code Review)