py/qstr: Special case qstr_find_strn for empty string. #12894
Merged
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
|
Code size report: |
Codecov Report
@@ Coverage Diff @@
## master #12894 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 158 158
Lines 20971 20972 +1
=======================================
+ Hits 20635 20636 +1
Misses 336 336
|
dpgeorge
reviewed
Nov 7, 2023
|
|
||
| int strncmp(const char *s1, const char *s2, size_t n) { | ||
| while (*s1 && *s2 && n-- > 0) { | ||
| while (n > 0 && *s1 && *s2) { |
Member
There was a problem hiding this comment.
Ha, this actually fixes the case when n=0 is passed in to this function, with both strings non-null.
C99 says that strncmp has UB for either string being NULL, so the current behavior is technically correct, but it's an easy fix to handle this case correctly. 7.1.4: "unless explicitly stated otherwise in the detailed description... if an argument to a function has ...null pointer.. the behavior is undefined". 7.21.1: "Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4". Also make the same change for the minimal version in bare-arm/lib.c. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>
This handles the case where an empty bytes/bytearray/str could pass in NULL as the str argument (with length zero). This would result in UB in strncmp. Even though our bare-metal implementation of strncmp handles this, best to avoid it for when we're using system strncmp. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <[email protected]>
dpgeorge
force-pushed
the
qstr-empty-str
branch
from
5b604e5 to
4212799
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Reported here: #12853 (comment)