| Submitter | Wolfgang Denk |
|---|---|
| Date | Nov. 28, 2011, 7:24 p.m. |
| Message ID | <1322508289-24908-1-git-send-email-wd@denx.de> |
| Download | mbox | patch |
| Permalink | /patch/128063/ |
| State | Accepted |
| Commit | a63d9652757605ec5f7104addc5d38bf10ba8671 |
| Headers | show |
Comments
On Monday 28 November 2011 14:24:49 Wolfgang Denk wrote: > common/menu.c used printf() in a number of places to print user > provided, constant strings (like the "title" string). printf() is > dangerous here for example in case the user unwittingly embeds some > '%' caracters that printf() would interpret as formatting and then > pick up random arguments. Use puts() instead. i'm not seeing this problem based on your patch below ... > --- a/common/menu.c > +++ b/common/menu.c > > - if (!m->item_data_print) > - printf("%s\n", item->key); > + putc(item->key); > + putc('\n'); item->key is not passed as the first arg, so % sequences would not get interpreted > - printf("%s:\n", m->title); > + puts(m->title); > + putc('\n'); same here > - printf("^C\n"); > + puts("^C\n"); this change makes sense, but not for any of the reasons cited in the changelog; this looks like a simple optimization ... -mike
Dear Mike Frysinger, In message <201111281810.03260.vapier@gentoo.org> you wrote: > > On Monday 28 November 2011 14:24:49 Wolfgang Denk wrote: > > common/menu.c used printf() in a number of places to print user > > provided, constant strings (like the "title" string). printf() is > > dangerous here for example in case the user unwittingly embeds some > > '%' caracters that printf() would interpret as formatting and then > > pick up random arguments. Use puts() instead. > > i'm not seeing this problem based on your patch below ... Yes, you are right. I was incorrectly extrapolating from another issue fixed elsewhere. > > - printf("^C\n"); > > + puts("^C\n"); > > this change makes sense, but not for any of the reasons cited in the > changelog; this looks like a simple optimization ... True. But d*mn, I have messed this up, and it sneaked into the master branch already. Sorry... Best regards, Wolfgang Denk
Patch
diff --git a/common/menu.c b/common/menu.c index f004823..ca1baef 100644 --- a/common/menu.c +++ b/common/menu.c @@ -87,10 +87,12 @@ static inline void *menu_item_print(struct menu *m, struct menu_item *item, void *extra) { - if (!m->item_data_print) - printf("%s\n", item->key); - else + if (!m->item_data_print) { + putc(item->key); + putc('\n'); + } else { m->item_data_print(item->data); + } return NULL; } @@ -117,8 +119,10 @@ static inline void *menu_item_destroy(struct menu *m, */ static inline void menu_display(struct menu *m) { - if (m->title) - printf("%s:\n", m->title); + if (m->title) { + puts(m->title); + putc('\n'); + } menu_items_iter(m, menu_item_print, NULL); } @@ -226,7 +230,7 @@ static inline int menu_interactive_choice(struct menu *m, void **choice) if (!choice_item) printf("%s not found\n", cbuf); } else { - printf("^C\n"); + puts("^C\n"); return -EINTR; } }
common/menu.c used printf() in a number of places to print user provided, constant strings (like the "title" string). printf() is dangerous here for example in case the user unwittingly embeds some '%' caracters that printf() would interpret as formatting and then pick up random arguments. Use puts() instead. We also omit the trailing ':' in the title line - if a user wants this, he can provide it as part of the title string. Signed-off-by: Wolfgang Denk <wd@denx.de> --- total: 0 errors, 0 warnings, 35 lines checked NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE /tmp/patch has no obvious style problems and is ready for submission. common/menu.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)