From 483f25751ae49c810456faf0bb7a375bc437df10 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Wed, 9 Jul 2014 21:13:58 +0200 Subject: [PATCH 1/1] Replace uses of variable-length arrays with try_malloc()/free(). --- TODO | 2 -- src/client/control.c | 10 +++++++--- src/client/draw_wins.c | 12 +++++++++--- src/client/wincontrol.c | 4 +++- src/client/windows.c | 14 +++++++++++--- src/common/readwrite.c | 15 +++++++++++---- src/common/rexit.c | 7 +++++-- src/common/try_malloc.c | 13 +++++++++++-- src/server/init.c | 9 ++++++--- src/server/io.c | 4 +++- src/server/run.c | 9 ++++++--- src/server/thing_actions.c | 8 ++++++-- src/server/things.c | 5 ++++- 13 files changed, 82 insertions(+), 30 deletions(-) diff --git a/TODO b/TODO index 3b60887..a5a87c3 100644 --- a/TODO +++ b/TODO @@ -4,8 +4,6 @@ IN GENERAL: - check for return values of *printf() -- be more strict and humble when allocating memory from the stack - - expand use of hardcoded_strings module(s) BOTH SERVER/CLIENT: diff --git a/src/client/control.c b/src/client/control.c index 43041ea..f97f5bd 100644 --- a/src/client/control.c +++ b/src/client/control.c @@ -2,22 +2,24 @@ #include "control.h" #include /* uint8_t, uint16_t, uint32_t, UINT32_MAX */ +#include /* free() */ #include /* sprintf() */ #include /* strlen() */ #include "../common/rexit.h" /* exit_err() */ +#include "../common/try_malloc.h" /* try_malloc() */ #include "interface_conf.h" /* reload_interface_conf(), save_interface_conf() */ #include "io.h" /* send() */ #include "keybindings.h" /* get_command_to_keycode(), get_keycode_to_command(), * mod_selected_keyb(), move_keyb_selection() */ -#include "map.h" /* for map_scroll(), map_center(), toggle_autofocus() */ +#include "map.h" /* map_scroll(), map_center(), toggle_autofocus() */ #include "wincontrol.h" /* shift_active_win(), resize_active_win(), * toggle_win_size_type(), toggle_window(), * cycle_active_win(), scroll_v_screen(), * toggle_linebreak_type(), toggle_winconfig() */ #include "windows.h" /* get_win_by_id() */ -#include "world.h" /* for global world */ +#include "world.h" /* world */ @@ -156,6 +158,7 @@ static uint8_t try_client_commands(struct Command * command) static uint8_t try_server_commands(struct Command * command) { + char * f_name = "try_server_commands()"; if (command->server_msg) { uint8_t arg = (uint8_t) command->arg; @@ -165,9 +168,10 @@ static uint8_t try_server_commands(struct Command * command) } uint8_t command_size = strlen(command->server_msg); uint8_t arg_size = 3; - char msg[command_size + 1 + arg_size + 1]; + char * msg = try_malloc(command_size + 1 + arg_size + 1, f_name); sprintf(msg, "%s %d", command->server_msg, arg); send(msg); + free(msg); return 1; } return 0; diff --git a/src/client/draw_wins.c b/src/client/draw_wins.c index 69278e8..b6a9c3e 100644 --- a/src/client/draw_wins.c +++ b/src/client/draw_wins.c @@ -178,12 +178,13 @@ static void add_line_long(struct Win * win, char * line, attr_t attri) static void add_line_compact(struct Win * win, char * line, attr_t attri, uint16_t * offset, uint8_t last) { + char * f_name = "add_line_compact()"; uint16_t y_start = win->winmap_size.y - (win->winmap_size.y > 0); try_resize_winmap(win, y_start + 1, win->frame_size.x); uint16_t len_line = strlen(line); char * separator = last ? "" : " / "; uint32_t len_line_new = len_line + strlen(separator); - char line_new[len_line_new]; + char * line_new = try_malloc(len_line_new, f_name); sprintf(line_new, "%s%s", line, separator); uint16_t x = 0; uint16_t y; @@ -204,6 +205,7 @@ static void add_line_compact(struct Win * win, char * line, attr_t attri, try_resize_winmap(win, y + 1 + 1, win->winmap_size.x); } } + free(line_new); *offset = x; } @@ -355,13 +357,15 @@ extern void draw_win_map(struct Win * win) extern void draw_win_info(struct Win * win) { + char * f_name = "draw_win_info()"; char * dsc_turn = "Turn: "; char * dsc_hitpoints = "\nHitpoints: "; uint16_t maxl = strlen(dsc_turn) + 5 + strlen(dsc_hitpoints) + 3; - char text[maxl + 1]; + char * text = try_malloc(maxl + 1, f_name); sprintf(text, "%s%d%s%d", dsc_turn, world.turn, dsc_hitpoints, world.player_lifepoints); add_text_with_linebreaks(win, text); + free(text); } @@ -455,6 +459,7 @@ extern void draw_winconf_keybindings(struct Win * win) extern void draw_winconf_geometry(struct Win * win) { + char * f_name = "draw_winconf_geometry()"; char * title = "Window's geometry:\n\n"; char * h_title = "Height to save: "; char h_value[6 + 1]; /* 6: int16_t value max strlen */ @@ -475,8 +480,9 @@ extern void draw_winconf_geometry(struct Win * win) + strlen(h_title) + strlen(h_value) + strlen(h_type) + strlen(w_title) + strlen(w_value) + strlen(w_type) + strlen(breaks_title) + strlen(breaks_type); - char text[text_size + 1]; + char * text = try_malloc(text_size + 1, f_name); sprintf(text, "%s%s%s%s%s%s%s%s%s", title, h_title, h_value, h_type, w_title, w_value, w_type, breaks_title, breaks_type); add_text_with_linebreaks(win, text); + free(text); } diff --git a/src/client/wincontrol.c b/src/client/wincontrol.c index 09cff03..e1a4fbd 100644 --- a/src/client/wincontrol.c +++ b/src/client/wincontrol.c @@ -328,10 +328,11 @@ extern void resize_active_win(char change) extern void shift_active_win(char dir) { + char * f_name = "shift_active_win()"; uint8_t len_order = strlen(world.winDB.order); if (1 < len_order) { - char tmp[len_order + 1]; + char * tmp = try_malloc(len_order + 1, f_name); tmp[len_order] = '\0'; uint8_t pos = get_win_pos_in_order(world.winDB.active); if ('f' == dir) @@ -362,6 +363,7 @@ extern void shift_active_win(char dir) world.winDB.order[pos - 1] = world.winDB.active; } } + free(tmp); update_wins(get_win_by_id(world.winDB.order[0])); } } diff --git a/src/client/windows.c b/src/client/windows.c index faf1f8a..7327fc2 100644 --- a/src/client/windows.c +++ b/src/client/windows.c @@ -13,6 +13,7 @@ #include /* free() */ #include /* memcpy(), strlen(), strnlen(), memset() */ #include "../common/rexit.h" /* exit_trouble(), exit_err() */ +#include "../common/try_malloc.h" /* try_malloc() */ #include "draw_wins.h" /* draw_winconf_geometry(), draw_winconf_keybindings(), * draw_win_inventory(), draw_win_info(), draw_win_log(), * draw_win_available_keybindings(), draw_win_map(), @@ -153,7 +154,8 @@ static void scroll_hint(struct yx_uint16 fsize, char dir, uint16_t dist, { dsc_space = fsize.y; } /* vv-- 10 = max strlen for uint16_t */ - char scrolldsc[1 + strlen(more) + 1 + 10 + 1 + strlen(unit) + 1 + 1]; + uint16_t size = 1 + strlen(more) + 1 + 10 + 1 + strlen(unit) + 1 + 1; + char * scrolldsc = try_malloc(size, "scroll_hint()"); sprintf(scrolldsc, " %d %s %s ", dist, more, unit); /* Decide on offset of the description text inside the scroll hint line. */ @@ -188,6 +190,7 @@ static void scroll_hint(struct yx_uint16 fsize, char dir, uint16_t dist, } mvwaddch(world.winDB.v_screen, start.y + draw_offset, start.x + q, c); } + free(scrolldsc); } @@ -207,6 +210,8 @@ static void winscroll_hint(struct Win * w, char dir, uint16_t dist) static void draw_win_borderlines(struct Win * w) { + char * f_name = "draw_win_borderlines()"; + /* Draw vertical and horizontal border lines. */ uint16_t y, x; for (y = w->start.y; y <= w->start.y + w->frame_size.y; y++) @@ -230,7 +235,7 @@ static void draw_win_borderlines(struct Win * w) offset = (w->frame_size.x - (strlen(w->title) + 2)) / 2; } /* +2 is for padding/decoration */ uint16_t length_visible = strnlen(w->title, w->frame_size.x - 2); - char title[length_visible + 3]; + char * title = try_malloc(length_visible + 3, f_name); char decoration = ' '; if (w->id == world.winDB.active) { @@ -240,6 +245,7 @@ static void draw_win_borderlines(struct Win * w) title[0] = title[length_visible + 1] = decoration; title[length_visible + 2] = '\0'; mvwaddstr(world.winDB.v_screen, w->start.y-1, w->start.x+offset, title); + free(title); } } @@ -431,9 +437,10 @@ extern void winch_called() extern void reset_windows_on_winch() { + char * f_name = "reset_windows_on_winch()"; endwin(); /* "[S]tandard way" to recalibrate ncurses post SIGWINCH, says */ refresh(); /* . */ - char tmp_order[strlen(world.winDB.order) + 1]; + char * tmp_order = try_malloc(strlen(world.winDB.order) + 1, f_name); sprintf(tmp_order, "%s", world.winDB.order); uint8_t i; char tmp_active = world.winDB.active; @@ -441,6 +448,7 @@ extern void reset_windows_on_winch() delwin(world.winDB.v_screen); make_v_screen_and_init_win_sizes(); for (i = 0; i < strlen(tmp_order); toggle_window(tmp_order[i]), i++); + free(tmp_order); world.winDB.active = tmp_active; } diff --git a/src/common/readwrite.c b/src/common/readwrite.c index 045bc4f..abc8ca8 100644 --- a/src/common/readwrite.c +++ b/src/common/readwrite.c @@ -6,24 +6,28 @@ #include /* FILE, fseek(), sprintf(), fgets(), fgetc(), ferror(), * fputc(), fwrite(), fclose(), fopen(), clearerr() */ +#include /* free() */ #include /* strlen() */ -#include /* for access(), unlink() */ +#include /* access(), unlink() */ #include "rexit.h" /* exit_err(), exit_trouble() */ +#include "try_malloc.h" /* try_malloc() */ extern FILE * try_fopen(char * path, char * mode, char * f) { + char * f_name = "try_fopen()"; char * msg1 = "Trouble in "; char * msg2 = " with fopen() (mode '"; char * msg3 = "') on path '"; char * msg4 = "'."; uint16_t size = strlen(msg1) + strlen(msg2) + strlen(msg3) + strlen(msg4) + strlen(f) + strlen(path) + strlen(mode) + 1; - char msg[size]; + char * msg = try_malloc(size, f_name); sprintf(msg, "%s%s%s%s%s%s%s", msg1, f, msg2, mode, msg3, path, msg4); FILE * file_p = fopen(path, mode); exit_err(NULL == file_p, msg); + free(msg); return file_p; } @@ -73,6 +77,7 @@ extern char * try_fgets(char * line, int linemax, FILE * file, char * f) extern void try_fclose_unlink_rename(FILE * file, char * p1, char * p2, char * f) { + char * f_name = "try_fclose_unlink_rename()"; try_fclose(file, f); char * msg1 = "Trouble in "; char * msg4 = "'."; @@ -81,17 +86,19 @@ extern void try_fclose_unlink_rename(FILE * file, char * p1, char * p2, char * msg2 = " with unlink() on path '"; uint16_t size = strlen(msg1) + strlen(msg2) + strlen(msg4) + strlen(f) + strlen(p2) + 1; - char msg[size]; + char * msg = try_malloc(size, f_name); sprintf(msg, "%s%s%s%s%s", msg1, f, msg2, p2, msg4); exit_err(unlink(p2), msg); + free(msg); } char * msg2 = " with rename() from '"; char * msg3 = "' to '"; uint16_t size = strlen(msg1) + strlen(f) + strlen(msg2) + strlen(p1) + strlen(msg3) + strlen(p2) + strlen(msg4) + 1; - char msg[size]; + char * msg = try_malloc(size, f_name); sprintf(msg, "%s%s%s%s%s%s%s", msg1, f, msg2, p1, msg3, p2, msg4); exit_err(rename(p1, p2), msg); + free(msg); } diff --git a/src/common/rexit.c b/src/common/rexit.c index facf63f..d8598cd 100644 --- a/src/common/rexit.c +++ b/src/common/rexit.c @@ -4,8 +4,9 @@ #include /* global errno */ #include /* uint16_t */ #include /* printf(), perror(), sprintf() */ -#include /* exit(), EXIT_FAILURE */ +#include /* exit(), free(), EXIT_FAILURE */ #include /* strlen() */ +#include "try_malloc.h" /* try_malloc() */ @@ -43,12 +44,14 @@ extern void exit_err(int err, char * msg) extern void exit_trouble(int err, char * parent, char * child) { + char * f_name = "exit_trouble()"; char * p1 = "Trouble in "; char * p2 = " with "; char * p3 = "."; uint16_t size = strlen(p1) + strlen(parent) + strlen(p2) + strlen(child) + strlen(p3) + 1; - char msg[size]; + char * msg = try_malloc(size, f_name); sprintf(msg, "%s%s%s%s%s", p1, parent, p2, child, p3); exit_err(err, msg); + free(msg); } diff --git a/src/common/try_malloc.c b/src/common/try_malloc.c index 2a21927..70392be 100644 --- a/src/common/try_malloc.c +++ b/src/common/try_malloc.c @@ -2,14 +2,23 @@ #include "try_malloc.h" #include /* for malloc */ +#include /* sprintf() */ +#include /* strlen() */ #include /* for size_t */ -#include "rexit.h" /* for exit_trouble() */ +#include "rexit.h" /* for exit_err() */ extern void * try_malloc(size_t size, char * f) { + char * prefix = "Trouble with malloc() in "; + char * msg = malloc(strlen(prefix) + strlen(f) + 1 + 1); + exit_err(NULL == msg, + "Trouble in try_malloc() with malloc() for error message string."); + int test = sprintf(msg, "%s%s.", prefix, f); + exit_err(test < 0, "Trouble in try_malloc() with sprintf()."); void * p = malloc(size); - exit_trouble(NULL == p, f, "malloc()"); + exit_err(NULL == p, msg); /* Bypass exit_trouble() calling try_malloc(). */ + free(msg); return p; } diff --git a/src/server/init.c b/src/server/init.c index 624d800..a91bd2a 100644 --- a/src/server/init.c +++ b/src/server/init.c @@ -43,7 +43,7 @@ static void replay_game() exit_err(access(s[PATH_RECORD], F_OK), "No record found to replay."); FILE * file = try_fopen(s[PATH_RECORD], "r", f_name); uint32_t linemax = textfile_width(file); - char line[linemax + 1]; + char * line = try_malloc(linemax + 1, f_name); while ( world.turn < world.replay && NULL != try_fgets(line, linemax + 1, file, f_name)) { @@ -61,6 +61,7 @@ static void replay_game() } } } + free(line); try_fclose(file, f_name); } @@ -170,7 +171,7 @@ extern void run_game() { FILE * file = try_fopen(path_savefile, "r", f_name); uint32_t linemax = textfile_width(file); - char line[linemax + 1]; + char * line = try_malloc(linemax + 1, f_name); while (NULL != try_fgets(line, linemax + 1, file, f_name)) { if (strlen(line) && strcmp("\n", line)) @@ -178,14 +179,16 @@ extern void run_game() obey_msg(line, 0); } } + free(line); try_fclose(file, f_name); } else { char * command = s[CMD_MAKE_WORLD]; - char msg[strlen(command) + 1 + 11 + 1]; + char * msg = try_malloc(strlen(command) + 1 + 11 + 1, f_name); sprintf(msg, "%s %d", command, (int) time(NULL)); obey_msg(msg, 1); + free(msg); } io_loop(); } diff --git a/src/server/io.c b/src/server/io.c index 4d1b609..bc9d877 100644 --- a/src/server/io.c +++ b/src/server/io.c @@ -180,7 +180,8 @@ static void read_file_into_queue() static void update_worldstate_file() { char * f_name = "update_worldstate_file()"; - char path_tmp[strlen(s[PATH_WORLDSTATE]) + strlen(s[PATH_SUFFIX_TMP]) + 1]; + uint16_t size = strlen(s[PATH_WORLDSTATE]) + strlen(s[PATH_SUFFIX_TMP]) + 1; + char * path_tmp = try_malloc(size, f_name); sprintf(path_tmp, "%s%s", s[PATH_WORLDSTATE], s[PATH_SUFFIX_TMP]); FILE * file = try_fopen(path_tmp, "w", f_name); struct Thing * player = get_player(); @@ -196,6 +197,7 @@ static void update_worldstate_file() try_fwrite(world.log, strlen(world.log), 1, file, f_name); } try_fclose_unlink_rename(file, path_tmp, s[PATH_WORLDSTATE], f_name); + free(path_tmp); set_cleanup_flag(CLEANUP_WORLDSTATE); char * dot = ".\n";; try_fwrite(dot, strlen(dot), 1, world.file_out, f_name); diff --git a/src/server/run.c b/src/server/run.c index 78b158b..9ab9f30 100644 --- a/src/server/run.c +++ b/src/server/run.c @@ -16,6 +16,7 @@ * textfile_width(), try_fputc() */ #include "../common/rexit.h" /* exit_trouble(), exit_err() */ +#include "../common/try_malloc.h" /* try_malloc() */ #include "ai.h" /* ai() */ #include "cleanup.h" /* set_cleanup_flag(), unset_cleanup_flag() */ #include "field_of_view.h" /* build_fov_map() */ @@ -198,7 +199,6 @@ static uint8_t parse_thing_manipulation(char * tok0, char * tok1) || parse_thing_command(tok0, tok1, t) || parse_val(tok0, tok1, s[CMD_ARGUMENT], '8', (char *)&t->arg) || parse_val(tok0, tok1, s[CMD_PROGRESS],'8',(char *)&t->progress) - || parse_val(tok0, tok1, s[CMD_LIFEPOINTS],'8', (char *) &t->lifepoints) || parse_position(tok0, tok1, t) @@ -309,23 +309,26 @@ static void turn_over() static void record_msg(char * msg) { char * f_name = "record_msg()"; - char path_tmp[strlen(s[PATH_RECORD]) + strlen(s[PATH_SUFFIX_TMP]) + 1]; + uint16_t size = strlen(s[PATH_RECORD]) + strlen(s[PATH_SUFFIX_TMP]) + 1; + char * path_tmp = try_malloc(size, f_name); sprintf(path_tmp, "%s%s", s[PATH_RECORD], s[PATH_SUFFIX_TMP]); FILE * file_tmp = try_fopen(path_tmp, "w", f_name); if (!access(s[PATH_RECORD], F_OK)) { FILE * file_read = try_fopen(s[PATH_RECORD], "r", f_name); uint32_t linemax = textfile_width(file_read); - char line[linemax + 1]; + char * line = try_malloc(linemax + 1, f_name); while (try_fgets(line, linemax + 1, file_read, f_name)) { try_fwrite(line, strlen(line), 1, file_tmp, f_name); } + free(line); try_fclose(file_read, f_name); } try_fwrite(msg, strlen(msg), 1, file_tmp, f_name); try_fputc('\n', file_tmp, f_name); try_fclose_unlink_rename(file_tmp, path_tmp, s[PATH_RECORD], f_name); + free(path_tmp); } diff --git a/src/server/thing_actions.c b/src/server/thing_actions.c index 2531ae9..bb8f585 100644 --- a/src/server/thing_actions.c +++ b/src/server/thing_actions.c @@ -113,6 +113,7 @@ static void update_log(char * text) static void actor_hits_actor(struct Thing * hitter, struct Thing * hitted) { + char * f_name = "actor_hits_actor()"; struct ThingType * tt_hitter = get_thing_type(hitter->type); struct ThingType * tt_hitted = get_thing_type(hitted->type); struct Thing * player = get_player(); @@ -129,9 +130,10 @@ static void actor_hits_actor(struct Thing * hitter, struct Thing * hitted) msg3 = tt_hitted->name; } uint8_t len = 1 + strlen(msg1) + 1 + strlen(msg2) + 1 + strlen(msg3) + 2; - char msg[len]; + char * msg = try_malloc(len, f_name); sprintf(msg, "\n%s %s %s.", msg1, msg2, msg3); update_log(msg); + free(msg); hitted->lifepoints--; if (0 == hitted->lifepoints) { @@ -168,6 +170,7 @@ static uint8_t match_dir(char d, char ** dsc_d, char match, char * dsc_match) static void playerbonus_move(char d, uint8_t passable) { + char * f_name = "playerbonus_move()"; char * dsc_dir = "north-east"; if ( match_dir(d, &dsc_dir, 'd', "east") || match_dir(d, &dsc_dir, 'c', "south-east") @@ -182,9 +185,10 @@ static void playerbonus_move(char d, uint8_t passable) { dsc_move = "You fail to move "; } - char msg[strlen(dsc_move) + strlen (dsc_dir) + 3]; + char * msg = try_malloc(strlen(dsc_move) + strlen (dsc_dir) + 3, f_name); sprintf(msg, "\n%s%s.", dsc_move, dsc_dir); update_log(msg); + free(msg); } diff --git a/src/server/things.c b/src/server/things.c index e0aa674..81d5931 100644 --- a/src/server/things.c +++ b/src/server/things.c @@ -186,12 +186,15 @@ extern struct Thing * get_player() extern struct ThingType * get_thing_type(uint8_t id) { + char * f_name = "get_thing_type()"; struct ThingType * tt = world.thing_types; for (; NULL != tt && id != tt->id; tt = tt->next); char * err_intro = "Requested thing type of unused ID "; - char err[strlen(err_intro) + 3 + 1 + 1]; + uint16_t size = strlen(err_intro) + 3 + 1 + 1; + char * err = try_malloc(size, f_name); sprintf(err, "%s%d.", err_intro, id); exit_err(NULL == tt, err); + free(err); return tt; } -- 2.30.2