X Tutup
Skip to content

GRIM: Suppress compiler warning on release build#6617

Merged
orgads merged 1 commit intoscummvm:masterfrom
orgads:grim-lua-strlen
May 14, 2025
Merged

GRIM: Suppress compiler warning on release build#6617
orgads merged 1 commit intoscummvm:masterfrom
orgads:grim-lua-strlen

Conversation

@orgads
Copy link
Contributor

@orgads orgads commented May 14, 2025

In function 'void Grim::addnchar(const char*, int32)',
    inlined from 'void Grim::addstr(const char*)' at engines/grim/lua/lstrlib.cpp:25:10,
    inlined from 'void Grim::str_rep()' at engines/grim/lua/lstrlib.cpp:75:9:
engines/grim/lua/lstrlib.cpp:20:16: warning: 'char* strncpy(char*, const char*, size_t)' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
   20 |         strncpy(b, s, n);
      |         ~~~~~~~^~~~~~~~~
In function 'void Grim::addstr(const char*)',
    inlined from 'void Grim::str_rep()' at engines/grim/lua/lstrlib.cpp:75:9:
engines/grim/lua/lstrlib.cpp:25:27: note: length computed here
   25 |         addnchar(s, strlen(s));
      |                     ~~~~~~^~~

@orgads orgads requested a review from aquadran May 14, 2025 16:24
@aquadran
Copy link
Member

I think it's not original intention to change this way. I think it will break engine.

Copy link
Member

@aquadran aquadran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can not be changed like that

In function 'void Grim::addnchar(const char*, int32)',
    inlined from 'void Grim::addstr(const char*)' at engines/grim/lua/lstrlib.cpp:25:10,
    inlined from 'void Grim::str_rep()' at engines/grim/lua/lstrlib.cpp:75:9:
engines/grim/lua/lstrlib.cpp:20:16: warning: 'char* strncpy(char*, const char*, size_t)' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
   20 |         strncpy(b, s, n);
      |         ~~~~~~~^~~~~~~~~
In function 'void Grim::addstr(const char*)',
    inlined from 'void Grim::str_rep()' at engines/grim/lua/lstrlib.cpp:75:9:
engines/grim/lua/lstrlib.cpp:25:27: note: length computed here
   25 |         addnchar(s, strlen(s));
      |                     ~~~~~~^~~
@orgads orgads force-pushed the grim-lua-strlen branch from b20f6ab to 524a742 Compare May 14, 2025 20:03
@orgads orgads requested a review from aquadran May 14, 2025 20:03
@orgads
Copy link
Contributor Author

orgads commented May 14, 2025

Ok, suppressed the warning instead.

@orgads orgads changed the title GRIM: Fix compiler warning on release build GRIM: Suppress compiler warning on release build May 14, 2025
@orgads orgads merged commit c613f2d into scummvm:master May 14, 2025
8 checks passed
@orgads orgads deleted the grim-lua-strlen branch May 14, 2025 20:08
@orgads
Copy link
Contributor Author

orgads commented May 15, 2025

@aquadran Can we replace strncpy with just memcpy? This will also be faster, and it looks like the input never contains null in the middle anyway.

@lephilousophe
Copy link
Member

I think it's the best way.
The original Lua interpreter did this at a later time when adding support for NUL characters in the strings.
But it's better to wait for @aquadran.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

X Tutup