From 96d1f29db8260ddd478391f87fb6299682f8ae55 Mon Sep 17 00:00:00 2001 From: Freeyorp Date: Mon, 15 Apr 2024 14:26:44 +0000 Subject: 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); ``` `#` instructs the preprocessor to transform `` 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 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::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if()->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::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if()->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::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if()->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::print(( # 4885 "script-fun.cpp" 3 4 stdout # 4885 "script-fun.cpp" ), (builtin_functions[st->stack->stack_datav[st->start].get_if()->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`. --- src/map/script-fun.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') 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 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; \ -- cgit v1.2.3-60-g2f50