-
-
Couldn't load subscription status.
- Fork 33.6k
src: fix StringSearch compiler warning #31798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
StringSearchBase has tables (int array members) that are used only for some search strategies, but g++-9 (at least) doesn't understand when they will or will not be used. Default-initialize them in the constructor to avoid this warning: In file included from ../../src/node_buffer.cc:29: ../../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = short unsigned int]’: ../../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized] 113 | return (this->*strategy_)(subject, index); | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ ../../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = unsigned char]’: ../../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized] 113 | return (this->*strategy_)(subject, index); | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
|
I believe the default initialization is a no-op, so no value will be written to memory, so that there is no performance impact. If that's not correct, some other strategy might be needed (if search construction is in critical path). |
It’s not – this will lead to zero-initialization: https://godbolt.org/z/2TcRgT |
From #26733 (comment). There is also #31532. |
Turn the `strategy_` method pointer into an enum-based static dispatch.
It's both safer and more secure (no chance of method pointer corruption)
and it helps GCC see that the shift and suffix tables it's complaining
about are unused in single char search mode.
Fixes the following warning:
../src/string_search.h:113:30: warning:
‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
return (this->*strategy_)(subject, index);
Fixes: nodejs#26733
Fixes: nodejs#31532
Fixes: nodejs#31798
|
Alternative solution: #31809 |
Turn the `strategy_` method pointer into an enum-based static dispatch.
It's both safer and more secure (no chance of method pointer corruption)
and it helps GCC see that the shift and suffix tables it's complaining
about are unused in single char search mode.
Fixes the following warning:
../src/string_search.h:113:30: warning:
‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
return (this->*strategy_)(subject, index);
Fixes: #26733
Refs: #31532
Refs: #31798
PR-URL: #31809
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Turn the `strategy_` method pointer into an enum-based static dispatch.
It's both safer and more secure (no chance of method pointer corruption)
and it helps GCC see that the shift and suffix tables it's complaining
about are unused in single char search mode.
Fixes the following warning:
../src/string_search.h:113:30: warning:
‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
return (this->*strategy_)(subject, index);
Fixes: #26733
Refs: #31532
Refs: #31798
PR-URL: #31809
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Turn the `strategy_` method pointer into an enum-based static dispatch.
It's both safer and more secure (no chance of method pointer corruption)
and it helps GCC see that the shift and suffix tables it's complaining
about are unused in single char search mode.
Fixes the following warning:
../src/string_search.h:113:30: warning:
‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
return (this->*strategy_)(subject, index);
Fixes: #26733
Refs: #31532
Refs: #31798
PR-URL: #31809
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
StringSearchBase has tables (int array members) that are used only for
some search strategies, but g++-9 (at least) doesn't understand when
they will or will not be used. Default-initialize them in the
constructor to avoid this warning:
Checklist