From 8644e5a347461232e2b2873af4b65add10e82800 Mon Sep 17 00:00:00 2001 From: mmdolze Date: Sat, 6 Jun 2009 14:48:48 +0000 Subject: [PATCH] Add checks for null pointer / failed malloc to menu code --- ChangeLog | 1 + server/commands/menu_commands.c | 4 ++ server/menu.c | 12 +++++ server/menuitem.c | 52 +++++++++++++++---- server/menuscreens.c | 89 +++++++++++++++++++++++---------- 5 files changed, 120 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4adc342..81b5cea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -72,6 +72,7 @@ v.0.5dev (ongoing development) * server: Fix interpretation of named screen priorities + new driver imonlcd for SoundGraph iMON LCD (Dean Harding, Jonathan Kyler) * MtxOrb driver: Fix backlight for old firmware + * Add checks for null pointer / failed malloc to menu code v.0.5.2 * fix switching on/off the Load screen in lcdproc client using the menu diff --git a/server/commands/menu_commands.c b/server/commands/menu_commands.c index 9ad9baa..a5d8621 100644 --- a/server/commands/menu_commands.c +++ b/server/commands/menu_commands.c @@ -120,6 +120,10 @@ menu_add_item_func(Client *c, int argc, char **argv) /* We need to create it */ report(RPT_INFO, "Client [%d] is using the menu", c->sock); c->menu = menu_create("_client_menu_", menu_commands_handler, c->name, c); + if (c->menu == NULL) { + sock_send_error(c->sock, "Cannot create menu\n"); + return 1; + } menu_add_item(main_menu, c->menu); } diff --git a/server/menu.c b/server/menu.c index c661c44..4e8b5d4 100644 --- a/server/menu.c +++ b/server/menu.c @@ -336,6 +336,18 @@ MenuItem *menu_find_item(Menu *menu, char *id, bool recursive) void menu_set_association(Menu *menu, void *assoc) { + /* + * assoc can currently be either a Screen or Driver, but there's + * no way to tell, so just display (data) if it's not NULL + */ + debug(RPT_DEBUG, "%s(menu=[%s], assoc=[%s])", __FUNCTION__, + ((menu != NULL) ? menu->id : "(null)"), + ((assoc != NULL) ? "(data)" : "(null)")); + + /* note that assoc is allowed to be NULL */ + if (menu == NULL) + return; + menu->data.menu.association = assoc; } diff --git a/server/menuitem.c b/server/menuitem.c index 35afada..83f0f7d 100644 --- a/server/menuitem.c +++ b/server/menuitem.c @@ -318,6 +318,11 @@ MenuItem *menuitem_create_slider(char *id, MenuEventFunc(*event_func), if (new_item != NULL) { new_item->data.slider.mintext = strdup(mintext); new_item->data.slider.maxtext = strdup(maxtext); + if (new_item->data.slider.mintext == NULL || + new_item->data.slider.maxtext == NULL) { + menuitem_destroy(new_item); + return NULL; + } new_item->data.slider.minvalue = minvalue; new_item->data.slider.maxvalue = maxvalue; new_item->data.slider.stepsize = stepsize; @@ -339,8 +344,12 @@ MenuItem *menuitem_create_numeric(char *id, MenuEventFunc(*event_func), if (new_item != NULL) { new_item->data.numeric.maxvalue = maxvalue; new_item->data.numeric.minvalue = minvalue; - new_item->data.numeric.edit_str = malloc(MAX_NUMERIC_LEN); new_item->data.numeric.value = value; + new_item->data.numeric.edit_str = malloc(MAX_NUMERIC_LEN); + if (new_item->data.numeric.edit_str == NULL) { + menuitem_destroy(new_item); + return NULL; + } } return new_item; @@ -366,15 +375,26 @@ MenuItem *menuitem_create_alpha(char *id, MenuEventFunc(*event_func), new_item->data.alpha.allow_noncaps = allow_noncaps; new_item->data.alpha.allow_numbers = allow_numbers; new_item->data.alpha.allowed_extra = strdup(allowed_extra); + if (new_item->data.alpha.allowed_extra == NULL) { + menuitem_destroy(new_item); + return NULL; + } new_item->data.alpha.value = malloc(maxlength + 1); - if (new_item->data.alpha.value != NULL) { - strncpy(new_item->data.alpha.value, value, maxlength); - new_item->data.alpha.value[maxlength] = 0; - } + if (new_item->data.alpha.value == NULL) { + menuitem_destroy(new_item); + return NULL; + } + + strncpy(new_item->data.alpha.value, value, maxlength); + new_item->data.alpha.value[maxlength] = 0; new_item->data.alpha.edit_str = malloc(maxlength + 1); - } + if (new_item->data.alpha.edit_str == NULL) { + menuitem_destroy(new_item); + return NULL; + } + } return new_item; } @@ -389,6 +409,9 @@ MenuItem *menuitem_create_ip(char *id, MenuEventFunc(*event_func), __FUNCTION__, id, event_func, text, v6, value); new_item = menuitem_create(MENUITEM_IP, id, event_func, text, client); + if (new_item == NULL) + return NULL; + new_item->data.ip.v6 = v6; ipinfo = (v6) ? &IPinfo[1] : &IPinfo[0]; @@ -425,6 +448,10 @@ MenuItem *menuitem_create_ip(char *id, MenuEventFunc(*event_func), } new_item->data.ip.edit_str = malloc(new_item->data.ip.maxlength + 1); + if (new_item->data.ip.edit_str == NULL) { + menuitem_destroy(new_item); + return NULL; + } return new_item; } @@ -1492,18 +1519,21 @@ LinkedList *tablist2linkedlist(char *strings) /* Alloc and copy substring */ new_s = malloc(len + 1); - strncpy(new_s, p, len); - new_s[len] = 0; + if (new_s != NULL) { + strncpy(new_s, p, len); + new_s[len] = 0; - LL_Push(list, new_s); + LL_Push(list, new_s); + } /* Go to next string */ p = tabptr + 1; } /* Add last string */ new_s = strdup(p); - LL_Push(list, new_s); - } + if (new_s != NULL) + LL_Push(list, new_s); + } return list; } diff --git a/server/menuscreens.c b/server/menuscreens.c index b6bb9a3..ba7714b 100644 --- a/server/menuscreens.c +++ b/server/menuscreens.c @@ -65,6 +65,9 @@ static void handle_enter(void); static void handle_successor(void); void menuscreen_switch_item(MenuItem *new_menuitem); void menuscreen_create_menu(void); +#ifdef LCDPROC_TESTMENUS +void menuscreen_create_testmenu(void); +#endif Menu *menuscreen_get_main(void); MenuEventFunc(heartbeat_handler); MenuEventFunc(backlight_handler); @@ -461,41 +464,36 @@ void menuscreen_create_menu(void) MenuItem *slider; Driver *driver; -#ifdef LCDPROC_TESTMENUS - MenuItem *test_item; - Menu *test_menu; - - char testiso[] = { - 'D', 'e', 'm', 'o', '\t', - /* #160 */ - 160, 161, 162, 163, 164, 165, 166, 167, '\t', - 168, 169, 170, 171, 172, 173, 174, 175, '\t', - 176, 177, 178, 179, 180, 181, 182, 183, '\t', - 184, 185, 186, 187, 188, 189, 190, 191, '\t', - /* #192 */ - 192, 193, 194, 195, 196, 197, 198, 199, '\t', - 200, 201, 202, 203, 204, 205, 206, 207, '\t', - 208, 209, 210, 211, 212, 213, 214, 215, '\t', - 216, 217, 218, 219, 220, 221, 222, 223, '\t', - /* #224 */ - 224, 225, 226, 227, 228, 229, 230, 231, '\t', - 232, 233, 234, 245, 236, 237, 238, 239, '\t', - 240, 241, 242, 243, 244, 245, 246, 247, '\t', - 248, 249, 250, 251, 252, 253, 254, 255, '\0' - }; -#endif /*LCDPROC_TESTMENUS*/ - debug(RPT_DEBUG, "%s()", __FUNCTION__); main_menu = menu_create("mainmenu", NULL, "LCDproc Menu", NULL); + if (main_menu == NULL) { + report(RPT_ERR, "%s: Cannot create main menu", __FUNCTION__); + return; + } options_menu = menu_create("options", NULL, "Options", NULL); + if (options_menu == NULL) { + report(RPT_ERR, "%s: Cannot create options menu", __FUNCTION__); + return; + } menu_add_item(main_menu, options_menu); #ifdef LCDPROC_TESTMENUS + /* TODO: + * Menu items in the screens menu currently have no functions assigned. + * Thefore only enable the menu for testing. If functions are available, + * this code should be outside the #ifdef. + */ screens_menu = menu_create("screens", NULL, "Screens", NULL); + if (screens_menu == NULL) { + report(RPT_ERR, "%s: Cannot create screens menu", __FUNCTION__); + return; + } menu_add_item(main_menu, screens_menu); -#endif /*LCDPROC_TESTMENUS*/ + + menuscreen_create_testmenu(); +#endif /* add option menu contents: * menu's client is NULL since we're in the server */ @@ -518,6 +516,11 @@ void menuscreen_create_menu(void) if (contrast_avail || brightness_avail) { /* menu's client is NULL since we're in the server */ driver_menu = menu_create(driver->name, NULL, driver->name, NULL); + if (driver_menu == NULL) { + report(RPT_ERR, "%s: Cannot create menu for driver %s", + __FUNCTION__, driver->name); + continue; + } menu_set_association(driver_menu, driver); menu_add_item(options_menu, driver_menu); if (contrast_avail) { @@ -542,9 +545,37 @@ void menuscreen_create_menu(void) } } } +} + +#ifdef LCDPROC_TESTMENUS +void menuscreen_create_testmenu(void) { + MenuItem *test_item; + Menu *test_menu; + + char testiso[] = { + 'D', 'e', 'm', 'o', '\t', + /* #160 */ + 160, 161, 162, 163, 164, 165, 166, 167, '\t', + 168, 169, 170, 171, 172, 173, 174, 175, '\t', + 176, 177, 178, 179, 180, 181, 182, 183, '\t', + 184, 185, 186, 187, 188, 189, 190, 191, '\t', + /* #192 */ + 192, 193, 194, 195, 196, 197, 198, 199, '\t', + 200, 201, 202, 203, 204, 205, 206, 207, '\t', + 208, 209, 210, 211, 212, 213, 214, 215, '\t', + 216, 217, 218, 219, 220, 221, 222, 223, '\t', + /* #224 */ + 224, 225, 226, 227, 228, 229, 230, 231, '\t', + 232, 233, 234, 245, 236, 237, 238, 239, '\t', + 240, 241, 242, 243, 244, 245, 246, 247, '\t', + 248, 249, 250, 251, 252, 253, 254, 255, '\0' + }; -#ifdef LCDPROC_TESTMENUS test_menu = menu_create("test", NULL, "Test menu", NULL); + if (test_menu == NULL) { + report(RPT_ERR, "%s: Cannot create test menu", __FUNCTION__); + return; + } menu_add_item(main_menu, test_menu); /* menu's client is NULL since we're in the server */ @@ -585,8 +616,8 @@ void menuscreen_create_menu(void) test_item = menuitem_create_ring("", NULL, "Charset", NULL, testiso, 0); menu_add_item(test_menu, test_item); -#endif /*LCDPROC_TESTMENUS*/ } +#endif /*LCDPROC_TESTMENUS*/ MenuEventFunc (heartbeat_handler) { @@ -683,6 +714,10 @@ menuscreen_add_screen(Screen *s) /* Create a menu entry for the screen */ m = menu_create(s->id, NULL, ((s->name != NULL) ? s->name : s->id), s->client); + if (m == NULL) { + report(RPT_ERR, "%s: Cannot create menu", __FUNCTION__); + return; + } menu_set_association(m, s); menu_add_item(screens_menu, m);