fix compile overflow warnings #2963
Open
+1
−1
Conversation
This file contains hidden or 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
Signed-off-by: lizhiqiang.sf <lizhiqiang.sf@bytedance.com>
Contributor
Author
|
my complice environment is： Perhaps you could take a look. cc @rjd15372 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #2963 +/- ##
============================================
+ Coverage 73.73% 73.88% +0.14%
============================================
Files 125 125
Lines 68911 68911
============================================
+ Hits 50813 50913 +100
+ Misses 18098 17998 -100
🚀 New features to boost your workflow:
|
Member
|
Same thing |
enjoy-binbin
approved these changes
Dec 23, 2025
ranshid
approved these changes
Dec 23, 2025
| sds *parts = sdssplitargs(shebang, &numparts); | ||
| sdsfree(shebang); | ||
| if (!parts || numparts == 0) { | ||
| if (!parts || numparts == 0 || sdslen(parts[0]) < 2) { |
Member
There was a problem hiding this comment.
The fix LGTM.
- Personally I find it more readable by trimming the shebang prefix first and then treat each argument separately eg:
--- a/src/eval.c
+++ b/src/eval.c
@@ -230,9 +230,8 @@ int evalExtractShebangFlags(sds body,
return C_ERR;
}
shebang_len = shebang_end - body;
- sds shebang = sdsnewlen(body, shebang_len);
+ sds shebang = sdsnewlen(body + 2, shebang_len - 2);
sds *parts = sdssplitargs(shebang, &numparts);
- sdsfree(shebang);
if (!parts || numparts == 0) {
if (err) *err = sdsnew("Invalid engine in script shebang");
sdsfreesplitres(parts, numparts);
@@ -240,9 +239,9 @@ int evalExtractShebangFlags(sds body,
}
if (out_engine) {
- size_t engine_name_len = sdslen(parts[0]) - 2;
+ size_t engine_name_len = sdslen(parts[0]);
*out_engine = zcalloc(engine_name_len + 1);
- valkey_strlcpy(*out_engine, parts[0] + 2, engine_name_len + 1);
+ valkey_strlcpy(*out_engine, parts[0], engine_name_len + 1);
}
script_flags &= ~SCRIPT_FLAG_EVAL_COMPAT_MODE;
- I would like if we had a len version of sdssplitargs which could help us avoid the unnecessary:
sds shebang = sdsnewlen(body, shebang_len);
as it is only created for the sdssplitargs does not except input string length and is tracking the null terminator.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Compilation warning in eval.c:244 when extracting shebang flags -
attempting to allocate 18446744073709551615 bytes (SIZE_MAX) due to
unsigned integer underflow.