- Notifications
You must be signed in to change notification settings - Fork13.3k
WString - assume c-string is a pointer to flash in all cases#9272
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
mcspr wants to merge19 commits intoesp8266:masterChoose a base branch frommcspr:string/cstr-pgmspace-funcs
base:master
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Uh oh!
There was an error while loading.Please reload this page.
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 presentexpect {w,}buffer to be present, things are catastrophic anywaysalso, redundant wbuffer check for free()introduce sys/ pgmspace.h, string.h and stdio.hconsistent signatures, avoid depending on newlib quirksallow 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() loopupdate replace() w/o '\0' assumptions, always use mem rangesdrop strstr usage and reuse loop blocksalso returns actual len diff after comparison is otherwise equal
979f9c0 to48f9fefCompareincorrectly using lastIndexOf, when should be using deduced search funcsince there is no reverse indexOf nor lastIndexOf allows end boundary,adjust buffer end pointer manually by swapping it w/ the found oneplus, reinit read ptr after possibly changing buffer() location
similar to replace() use, we already know there is nothing pastpreviously found needle ptr
Sign up for freeto 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.
Uh oh!
There was an error while loading.Please reload this page.
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)