summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFreeyorp <TheFreeYorp+git@gmail.com>2024-04-15 14:26:44 +0000
committerLed Mitz <smoothshifter@tuta.io>2024-04-22 22:15:30 +0000
commit96d1f29db8260ddd478391f87fb6299682f8ae55 (patch)
treea735e39d24f2ee534b1c76d4977ecedda44ae5e8
parent000cdca878911759643e18a9648c587e9730fa91 (diff)
downloadtmwa-96d1f29db8260ddd478391f87fb6299682f8ae55.tar.gz
tmwa-96d1f29db8260ddd478391f87fb6299682f8ae55.tar.bz2
tmwa-96d1f29db8260ddd478391f87fb6299682f8ae55.tar.xz
tmwa-96d1f29db8260ddd478391f87fb6299682f8ae55.zip
script_nullpo_end: Change error printf fmt -> arg
PR !270 is an ongoing investigation into outstanding compiler warnings. One such warning is `warning: format '%s' expects a matching 'char*' argument` from `script_nullpo_end(nd, STRPRINTF("no such npc: '%s'"_fmt, name));` in `src/map/script-fun.cpp`. No such warning is emitted from a similar line in `src/map/atcommand.cpp`: `AString output = STRPRINTF("Jump to %s"_fmt, npc);`. `script_nullpo_end` is a macro, rather than a function. `error` is passed in verbatim by the preprocessor. The macro definition of `script_nullpo_end` includes lines such as the following: ```cpp PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name); ``` `#<variable>` instructs the preprocessor to transform `<variable>` into a literal string. In this case, the string literal: ```cpp "STRPRINTF(\"no such npc: '%s'\"_fmt, name)" ``` So the entire line partly resolves to something like: ```cpp printf("script:%s: STRPRINTF(\"no such npc: '%s'\"_fmt, name) @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name); ``` Once the compiler sees this, it throws out the warning seen here, because printf is seeing three placeholders in the formatter, but is only given two value arguments. Checking with `gcc -E`, the full expansion is as follows: ```cpp if (nullpo_chk("script-fun.cpp", 4885, __PRETTY_FUNCTION__, nd)) { if (st->oid) { dumb_ptr<npc_data> nullpo_nd = map_id_is_npc(st->oid); if (nullpo_nd && nullpo_nd->name) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " @ %s\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name), nullpo_nd->name); }); } else if (nullpo_nd) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (unnamed npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } st->state = ScriptEndState::END; return; }; ``` Note the string literal: `"STRPRINTF(\"no such npc: '%s'\"_fmt, npc)"`. `script_nullpo_end` currently can't take as its argument for `error` any expression which, when stringified, contains something which `printf` will interpret as its formatter expecting another argument. This appears to have been broken since it was first introduced in https://git.themanaworld.org/legacy/tmwa/-/commit/594eafd4f231a897cbefd since the first revision implicitly had these requirements, and also introduced usage which didn't meet them. This rewrites each PRINTF line in `script_nullpo_end` from the template ```cpp PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name); ``` to something like ```cpp PRINTF("script:%s: %s @ %s\n"_fmt, BUILTIN_NAME(), error, nullpo_nd->name); ``` This means that instead of an expression being stringified, error is interpreted as an expression that resolves (or decays) to a string. This seems consistent with all current usage of `script_nullpo_end`.
-rw-r--r--src/map/script-fun.cpp8
1 files changed, 4 insertions, 4 deletions
diff --git a/src/map/script-fun.cpp b/src/map/script-fun.cpp
index 2be0333..deb781a 100644
--- a/src/map/script-fun.cpp
+++ b/src/map/script-fun.cpp
@@ -80,14 +80,14 @@ namespace map
if (st->oid) { \
dumb_ptr<npc_data> nullpo_nd = map_id_is_npc(st->oid); \
if (nullpo_nd && nullpo_nd->name) { \
- PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name); \
+ PRINTF("script:%s: %s @ %s\n"_fmt, BUILTIN_NAME(), error, nullpo_nd->name); \
} else if (nullpo_nd) { \
- PRINTF("script:%s: " #error " (unnamed npc)\n"_fmt, BUILTIN_NAME()); \
+ PRINTF("script:%s: %s (unnamed npc)\n"_fmt, BUILTIN_NAME(), error); \
} else { \
- PRINTF("script:%s: " #error " (no npc)\n"_fmt, BUILTIN_NAME()); \
+ PRINTF("script:%s: %s (no npc)\n"_fmt, BUILTIN_NAME(), error); \
} \
} else { \
- PRINTF("script:%s: " #error " (no npc)\n"_fmt, BUILTIN_NAME()); \
+ PRINTF("script:%s: %s (no npc)\n"_fmt, BUILTIN_NAME(), error); \
} \
st->state = ScriptEndState::END; \
return; \