Misc tweaks #492
Closed
Misc tweaks #492
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Please GCC:
In file included from /usr/include/php/20220829/Zend/zend.h:30,
from /usr/include/php/20220829/main/php.h:31,
from /usr/include/php/20220829/main/SAPI.h:20,
from src/php_snuffleupagus.h:37,
from src/sp_ifilter.c:1:
src/sp_pcre_compat.h: In function 'sp_regexp_compile':
src/sp_pcre_compat.h:38:36: warning: '__zend_calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args]
38 | sp_regexp *ret = pecalloc(sizeof(sp_regexp), 1, 1);
| ^~~~~~~~~
/usr/include/php/20220829/Zend/zend_alloc.h:199:72: note: in definition of macro 'pecalloc'
199 | #define pecalloc(nmemb, size, persistent) ((persistent)?__zend_calloc((nmemb), (size)):ecalloc((nmemb), (size)))
| ^~~~~
src/sp_pcre_compat.h:38:36: note: earlier argument should specify number of elements, later size of each element
38 | sp_regexp *ret = pecalloc(sizeof(sp_regexp), 1, 1);
| ^~~~~~~~~
/usr/include/php/20220829/Zend/zend_alloc.h:199:72: note: in definition of macro 'pecalloc'
199 | #define pecalloc(nmemb, size, persistent) ((persistent)?__zend_calloc((nmemb), (size)):ecalloc((nmemb), (size)))
| ^~~~~
Avoid the configure step each time during development.
Use the special value void as parameter for functions taking nor argument.
Avoid missing prototype warnings by declaring variables and functions that are only used in a single file static.
Adjusts casts to void dropping const qualifiers. This helps to avoid mistakes, e.g. modifying string literals. Also use size_t for length, similar to the upstream php interfaces.
Please static analyzers.
jvoisin
requested changes
May 29, 2024
| @@ -172,7 +172,6 @@ static zval *get_unknown_type(const char *restrict value, zval *zvalue, | |||
| const sp_tree *const tree, bool is_param) { | |||
| if (ce) { | |||
| zvalue = get_entry_hashtable(&ce->constants_table, value, strlen(value)); | |||
| ce = NULL; | |||
There was a problem hiding this comment.
Did you check if ce isn't used in the callstack?
There was a problem hiding this comment.
ce is a local variable to this function, so an assignment here has no effect in any caller.
There was a problem hiding this comment.
ce is passed as a parameter to the function, it's not a local variable, is it?
There was a problem hiding this comment.
ce is a local copy of the address, so assigning NULL here has no effect to the address in the variable in the caller.
Annotate the common logging function sp_log_msgf() with the format attribute so compilers can check the used format string and passed arguments for discrepancies. Adjust the lineno printing by using %zu and the type size_t consistently.
src/sp_unserialize.c: In function 'zif_sp_unserialize':
src/sp_unserialize.c:131:15: warning: unused variable 'orig_handler' [-Wunused-variable]
131 | zif_handler orig_handler = zend_hash_str_find_ptr(SPG(sp_internal_functions_hook), ZEND_STRL("unserialize"));
| ^~~~~~~~~~~~
jvoisin
reviewed
May 31, 2024
| /src/tests/disable_function/test.bla | ||
| /src/tests/disable_function/test.meh | ||
| /src/tests/disable_function/test.sim | ||
| /src/tests/eval_blacklist/test.bla | ||
|
|
There was a problem hiding this comment.
Those files should be deleted by the testsuite, instead of being ignored here.
There was a problem hiding this comment.
I tried using --CLEAN--, but it seems php does not run the .clean.php snippet with the supplied configuration from --INI-- and thus the test borks, since the cleanup output is not empty but contains No configuration specified via sp.configuration_file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.