From 6fea5f5e1a1bbc6cfce8e6377cb7a1de0fa9f6f0 Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Mon, 29 Mar 2021 17:27:06 +0900 Subject: [PATCH] [build] Add -Wformat-non-literal option While this caused some trouble for pr_strings and configurable strftime (evil hacks abound), it's the result of discovering an ancient (from maybe as early as 2004, definitely before 2012) bug in qwaq's printing that somehow got past months of trial-by-fire testing (origin understood thanks to the warning finding it). --- config.d/compiling.m4 | 1 + libs/gamecode/pr_debug.c | 2 +- libs/gamecode/pr_strings.c | 10 ++-- libs/util/plugin.c | 73 ++++++++++++++-------------- libs/video/renderer/vulkan/vkparse.c | 3 +- nq/source/sbar.c | 13 +++-- qw/include/cl_main.h | 7 ++- qw/source/cl_main.c | 6 --- qw/source/sbar.c | 14 +++--- qw/source/sv_send.c | 5 +- qw/source/sv_user.c | 9 ++-- ruamoko/qwaq/builtins/main.c | 5 +- ruamoko/qwaq/builtins/qwaq-bi.c | 5 +- 13 files changed, 78 insertions(+), 75 deletions(-) diff --git a/config.d/compiling.m4 b/config.d/compiling.m4 index f5042ae0f..b432c1709 100644 --- a/config.d/compiling.m4 +++ b/config.d/compiling.m4 @@ -221,6 +221,7 @@ QF_CC_OPTION(-Wsuggest-attribute=pure) QF_CC_OPTION(-Wsuggest-attribute=const) QF_CC_OPTION(-Wsuggest-attribute=noreturn) QF_CC_OPTION(-Wsuggest-attribute=format) +QF_CC_OPTION(-Wformat-nonliteral) AC_ARG_ENABLE(coverage, [ --enable-coverage Enable generation of data for gcov]) diff --git a/libs/gamecode/pr_debug.c b/libs/gamecode/pr_debug.c index 598c2fa18..3ef208442 100644 --- a/libs/gamecode/pr_debug.c +++ b/libs/gamecode/pr_debug.c @@ -352,7 +352,7 @@ parse_expression (progs_t *pr, const char *expr, int conditional) } error: if (es->error) { - Sys_Printf (es->error); + Sys_Printf ("%s\n", es->error); } Script_Delete (es); return d; diff --git a/libs/gamecode/pr_strings.c b/libs/gamecode/pr_strings.c index 82e9540da..c0a0df833 100644 --- a/libs/gamecode/pr_strings.c +++ b/libs/gamecode/pr_strings.c @@ -712,22 +712,24 @@ PR_FreeTempStrings (progs_t *pr) pr->pr_xtstr = 0; } +#define hasprintf ((char *(*)(dstring_t *, const char *, ...))dasprintf) + #define PRINT(t) \ switch ((doWidth << 1) | doPrecision) { \ case 3: \ - dasprintf (result, tmp->str, current->minFieldWidth, \ + hasprintf (result, tmp->str, current->minFieldWidth, \ current->precision, current->data.t##_var); \ break; \ case 2: \ - dasprintf (result, tmp->str, current->minFieldWidth, \ + hasprintf (result, tmp->str, current->minFieldWidth, \ current->data.t##_var); \ break; \ case 1: \ - dasprintf (result, tmp->str, current->precision, \ + hasprintf (result, tmp->str, current->precision, \ current->data.t##_var); \ break; \ case 0: \ - dasprintf (result, tmp->str, current->data.t##_var); \ + hasprintf (result, tmp->str, current->data.t##_var); \ break; \ } diff --git a/libs/util/plugin.c b/libs/util/plugin.c index f625ddd15..9505475fc 100644 --- a/libs/util/plugin.c +++ b/libs/util/plugin.c @@ -52,11 +52,12 @@ # endif #endif +#include "QF/cmd.h" #include "QF/cvar.h" #include "QF/hash.h" #include "QF/plugin.h" #include "QF/sys.h" -#include "QF/cmd.h" +#include "QF/va.h" #include "QF/plugin/general.h" @@ -150,29 +151,28 @@ pi_open_lib (const char *name, int global_syms) return dlhand; } -static void -pi_realname (char *realname, int size, const char *type, const char *name) +static const char * +pi_realname (const char *type, const char *name) { #if defined(HAVE_DLOPEN) - const char *format = "%s/%s_%s.so"; + return va (0, "%s/%s_%s.so", fs_pluginpath->string, type, name); #elif defined(_WIN32) - const char *format = "%s/%s_%s.dll"; + return va (0, "%s/%s_%s.dll", fs_pluginpath->string, type, name); #else - const char *format = "No shared library support. FIXME"; + return "No shared library support. FIXME"; #endif - - snprintf (realname, size, format, fs_pluginpath->string, type, name); } -static void -pi_info_name (char *info_name, int size, const char *type, const char *name) +static const char * +pi_info_name (const char *type, const char *name) { - if (type && name) - snprintf (info_name, size, "%s_%s_PluginInfo", type, name); - else if (type) - snprintf (info_name, size, "%s_PluginInfo", type); - else - snprintf (info_name, size, "PluginInfo"); + if (type && name) { + return va (0, "%s_%s_PluginInfo", type, name); + } else if (type) { + return va (0, "%s_PluginInfo", type); + } else { + return "PluginInfo"; + } } static void @@ -208,7 +208,7 @@ PI_Plugin_Load_f (void) static void PI_Plugin_Unload_f (void) { - char plugin_name[1024]; + const char *plugin_name; loaded_plugin_t *lp; plugin_t *pi; @@ -218,8 +218,8 @@ PI_Plugin_Unload_f (void) } // try to locate the plugin - snprintf (plugin_name, sizeof (plugin_name), "%s_%s", Cmd_Argv(1), - Cmd_Argv(2)); + plugin_name = va (0, "%s_%s", Cmd_Argv(1), Cmd_Argv(2)); + lp = Hash_Find (loaded_plugins, plugin_name); if (lp) { pi = lp->plugin; @@ -268,23 +268,27 @@ PI_Shutdown (void) VISIBLE plugin_t * PI_LoadPlugin (const char *type, const char *name) { - char realname[4096]; - char plugin_name[1024]; - char plugin_info_name[1024]; - char *tmpname; - void *dlhand = NULL; - plugin_t *plugin = NULL; - P_PluginInfo plugin_info = NULL; - plugin_list_t *pl; + const char *realname = 0; + const char *plugin_name = 0; + const char *plugin_info_name = 0; + const char *tmpname; + void *dlhand = 0; + plugin_t *plugin = 0; + P_PluginInfo plugin_info = 0; + plugin_list_t *pl; loaded_plugin_t *lp; if (!name) return NULL; - tmpname = strrchr (name, '/'); // Get the base name, don't allow paths + // Get the base name, don't allow paths + tmpname = strrchr (name, '/'); + if (tmpname) { + name = tmpname + 1; // skip over '/' + } // Build the plugin name - snprintf (plugin_name, sizeof (plugin_name), "%s_%s", type, name); + plugin_name = va (0, "%s_%s", type, name); // make sure we're not already loaded lp = Hash_Find (loaded_plugins, plugin_name); @@ -299,8 +303,7 @@ PI_LoadPlugin (const char *type, const char *name) } if (!plugin_info) { // Build the path to the file to load - pi_realname (realname, sizeof (realname), type, - (tmpname ? tmpname + 1 : name)); + realname = pi_realname (type, name); if (!(dlhand = pi_open_lib (realname, 0))) { // lib not found @@ -310,15 +313,13 @@ PI_LoadPlugin (const char *type, const char *name) } // Build the plugin info name as $type_$name_PluginInfo - pi_info_name (plugin_info_name, sizeof (plugin_info_name), type, name); + plugin_info_name = pi_info_name (type, name); if (!(plugin_info = pi_get_symbol (dlhand, plugin_info_name))) { // Build the plugin info name as $type_PluginInfo - pi_info_name (plugin_info_name, sizeof (plugin_info_name), - type, 0); + plugin_info_name = pi_info_name (type, 0); if (!(plugin_info = pi_get_symbol (dlhand, plugin_info_name))) { // Build the plugin info name as PluginInfo - pi_info_name (plugin_info_name, sizeof (plugin_info_name), - 0, 0); + plugin_info_name = pi_info_name (0, 0); if (!(plugin_info = pi_get_symbol (dlhand, plugin_info_name))) { // info function not found pi_close_lib (dlhand); diff --git a/libs/video/renderer/vulkan/vkparse.c b/libs/video/renderer/vulkan/vkparse.c index 798cd98f9..1100024fd 100644 --- a/libs/video/renderer/vulkan/vkparse.c +++ b/libs/video/renderer/vulkan/vkparse.c @@ -240,14 +240,13 @@ parse_reference (const plitem_t *item, const char *type, plitem_t *messages, parsectx_t *pctx) { exprctx_t ectx = *pctx->ectx; - vulkan_ctx_t *ctx = pctx->vctx; plitem_t *refItem = 0; exprval_t result = { &cexpr_plitem, &refItem }; ectx.symtab = 0; ectx.result = &result; const char *name = PL_String (item); if (cexpr_eval_string (name, &ectx)) { - PL_Message (messages, item, va (ctx->va_ctx, "not a %s reference", type)); + PL_Message (messages, item, "not a %s reference", type); return 0; } return refItem; diff --git a/nq/source/sbar.c b/nq/source/sbar.c index 506cb7fa3..1a0261db2 100644 --- a/nq/source/sbar.c +++ b/nq/source/sbar.c @@ -1093,23 +1093,22 @@ draw_overlay (view_t *view) static void draw_time (view_t *view) { - struct tm *local = NULL; + struct tm *local = 0; time_t utc = 0; - const char *timefmt = NULL; char st[80]; //FIXME: overflow // Get local time - utc = time (NULL); + utc = time (0); local = localtime (&utc); if (hud_time->int_val == 1) { // Use international format - timefmt = "%k:%M"; + strftime (st, sizeof (st), "%k:%M", local); + draw_string (view, 8, 0, st); } else if (hud_time->int_val >= 2) { // US AM/PM display - timefmt = "%l:%M %P"; + strftime (st, sizeof (st), "%l:%M %P", local); + draw_string (view, 8, 0, st); } - strftime (st, sizeof (st), timefmt, local); - draw_string (view, 8, 0, st); } static void diff --git a/qw/include/cl_main.h b/qw/include/cl_main.h index f7cd463f1..16727deea 100644 --- a/qw/include/cl_main.h +++ b/qw/include/cl_main.h @@ -44,7 +44,12 @@ qboolean CL_DemoBehind(void); void CL_BeginServerConnect(void); -extern char emodel_name[], pmodel_name[], prespawn_name[], modellist_name[], soundlist_name[]; +#define emodel_name "emodel" +#define pmodel_name "pmodel" +#define prespawn_name "prespawn %i 0 %i" +#define modellist_name "modellist %i %i" +#define soundlist_name "soundlist %i %i" + extern struct cvar_s *cl_timeframes; extern struct cvar_s *cl_predict_players; diff --git a/qw/source/cl_main.c b/qw/source/cl_main.c index 15617aea3..411cfcbc0 100644 --- a/qw/source/cl_main.c +++ b/qw/source/cl_main.c @@ -224,12 +224,6 @@ jmp_buf host_abort; char *server_version = NULL; // version of server we connected to -char emodel_name[] = "emodel"; -char pmodel_name[] = "pmodel"; -char prespawn_name[] = "prespawn %i 0 %i"; -char modellist_name[] = "modellist %i %i"; -char soundlist_name[] = "soundlist %i %i"; - extern cvar_t *hud_scoreboard_uid; static netadr_t cl_cmd_packet_address; diff --git a/qw/source/sbar.c b/qw/source/sbar.c index 4a517887e..0514a9327 100644 --- a/qw/source/sbar.c +++ b/qw/source/sbar.c @@ -1555,23 +1555,21 @@ draw_miniteam (view_t *view) static void draw_time (view_t *view) { - struct tm *local = NULL; + struct tm *local = 0; time_t utc = 0; - const char *timefmt = NULL; char st[80]; // Get local time - utc = time (NULL); + utc = time (0); local = localtime (&utc); if (hud_time->int_val == 1) { // Use international format - timefmt = "%k:%M"; + strftime (st, sizeof (st), "%k:%M", local); + draw_string (view, 8, 0, st); } else if (hud_time->int_val >= 2) { // US AM/PM display - timefmt = "%l:%M %P"; + strftime (st, sizeof (st), "%l:%M %P", local); + draw_string (view, 8, 0, st); } - - strftime (st, sizeof (st), timefmt, local); - draw_string (view, 8, 8, st); } static void diff --git a/qw/source/sv_send.c b/qw/source/sv_send.c index 0948e6f18..b89438131 100644 --- a/qw/source/sv_send.c +++ b/qw/source/sv_send.c @@ -164,6 +164,9 @@ find_userid (const char *name) return 0; } +#define hstrftime ((size_t (*)(char *s, size_t, const char *, \ + const struct tm*))strftime) + /* SV_Printf @@ -229,7 +232,7 @@ SV_Print (const char *fmt, va_list args) if (timestamps) { mytime = time (NULL); local = localtime (&mytime); - strftime (msg3, sizeof (msg3), sv_timefmt->string, local); + hstrftime (msg3, sizeof (msg3), sv_timefmt->string, local); dsprintf (msg2, "%s%s", msg3, msg->str); } else { diff --git a/qw/source/sv_user.c b/qw/source/sv_user.c index dd1fe7bfc..a3760ad03 100644 --- a/qw/source/sv_user.c +++ b/qw/source/sv_user.c @@ -820,7 +820,7 @@ SV_Say (qboolean team) { char *i, *p; dstring_t *text; - const char *t1 = 0, *t2, *type, *fmt; + const char *t1 = 0, *t2, *type; client_t *client; int tmp, j, cls = 0; sizebuf_t *dbuf; @@ -903,16 +903,15 @@ SV_Say (qboolean team) text = dstring_new (); if (host_client->spectator && (!sv_spectalk->int_val || team)) { - fmt = "[SPEC] %s: "; type = "2"; + dsprintf (text, "[SPEC] %s: ", host_client->name); } else if (team) { - fmt = "(%s): "; type = "1"; + dsprintf (text, "(%s): ", host_client->name); } else { - fmt = "%s: "; type = "0"; + dsprintf (text, "%s: ", host_client->name); } - dsprintf (text, fmt, host_client->name); if (sv_chat_e->func) GIB_Event_Callback (sv_chat_e, 2, va (0, "%i", host_client->userid), p, diff --git a/ruamoko/qwaq/builtins/main.c b/ruamoko/qwaq/builtins/main.c index 7e4e7f5b4..8c8ec0f69 100644 --- a/ruamoko/qwaq/builtins/main.c +++ b/ruamoko/qwaq/builtins/main.c @@ -352,8 +352,9 @@ bi_printf (progs_t *pr) dstring_t *dstr = dstring_new (); PR_Sprintf (pr, dstr, "bi_printf", fmt, count, args); - if (dstr->str) - Sys_Printf (dstr->str, stdout); + if (dstr->str) { + Sys_Printf ("%s", dstr->str); + } dstring_delete (dstr); } diff --git a/ruamoko/qwaq/builtins/qwaq-bi.c b/ruamoko/qwaq/builtins/qwaq-bi.c index a78dd7763..d3db5917a 100644 --- a/ruamoko/qwaq/builtins/qwaq-bi.c +++ b/ruamoko/qwaq/builtins/qwaq-bi.c @@ -89,8 +89,9 @@ bi_printf (progs_t *pr) dstring_t *dstr = dstring_new (); PR_Sprintf (pr, dstr, "bi_printf", fmt, count, args); - if (dstr->str) - Con_Printf (dstr->str, stdout); + if (dstr->str) { + Con_Printf ("%s", dstr->str); + } dstring_delete (dstr); }