WString - assume c-string is a pointer to flash in all cases #9272
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
consistent behaviour when accepting cstr input
expect empty cstr for simple methods, avoid doing it w/ length present
expect {w,}buffer to be present, things are catastrophic anyways
also, redundant wbuffer check for free()
introduce sys/ pgmspace.h, string.h and stdio.h consistent signatures, avoid depending on newlib quirks allow strstr & strstr_P to be the same thing
wrappers expect length, thus no need to look for '\0' only missing piece is case-insensitive memcmp (memicmp?) since implementation requires it, manually do the tolower() loop update replace() w/o '\0' assumptions, always use mem ranges drop strstr usage and reuse loop blocks also returns actual len diff after comparison is otherwise equal
mcspr
force-pushed
the
string/cstr-pgmspace-funcs
branch
from
979f9c0 to
48f9fef
Compare
incorrectly using lastIndexOf, when should be using deduced search func since there is no reverse indexOf nor lastIndexOf allows end boundary, adjust buffer end pointer manually by swapping it w/ the found one plus, reinit read ptr after possibly changing buffer() location
similar to replace() use, we already know there is nothing past previously found needle ptr
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Currently, it is mixed. Some methods allow it, some fall back to newlib or ROM funcs. Some converted c-string to String(). Or, some caused implicit conversion nuance (like String(const char*) and operator=(const char*))
Instead, try to pass data through as c-string + its length and minimize String() conversions. This also drops most of the
__FlashStringHelperoverloads in favour of forcibly casting it to c-string.Internals also check for flash/non-flash to select an appropriate copy func from newlib or rom (...or one internal memcmp that is case-insensitive). Methods previously missing length argument now try to utilize it internally and thus use string.h mem* funcs instead of str*
Also refactored some long standing weirdness w/ null-checks of source strings & buffer()s
resolve #9269
@gorlov-ai would you mind checking whether this fixes your code that crashed?
(preferably clone this whole tree, but c/p just the WString{.h,.cpp} files may also work)