Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream::find() not working for values above 0x7F #119

Open
pnjun opened this issue Sep 8, 2020 · 1 comment
Open

Stream::find() not working for values above 0x7F #119

pnjun opened this issue Sep 8, 2020 · 1 comment

Comments

@pnjun
Copy link

pnjun commented Sep 8, 2020

I found out that the Stream::find() (and all related functions, findUntil() etc), does not work when the sequence to be found contains values above 0x7F. I believe this is because the function uses char instead of uint8_t.

Example, using these definitions:

char     a = 0xff;
uint8_t b = 0xff;

a == b is false, and therefore the find() method does not return a match when supplied a uint8_t[] array containing values above 0x7F, since the array gets casted to char. Changing the declaration of find() to uint8_t should solve the issue.

I don't know if ignoring values above 0x7F is the intended behaviour, but if that is the case I think the documentation should be more specific and explain this case better. In any case, if the current behaviour is the intended one, i do not see the reason behind it.

First time reporting something on Arduino, sorry if I overlooked something obvious.

@matthijskooijman
Copy link
Collaborator

Sounds like sloppy typing indeed, would be good to fix. I haven't looked closely, but I think that using char and char* in the public API is ok (since that's the common typing for a string), but internally, the code should make sure to either convert the target to uint8_t, or convert the thing to compare with to char too.

Did you try changing the typing to confirm that this indeeds fixes the problem?

Note that development on the non-arch-specific code happens at ArduinoCore-API repo now, so I'll transfer this issue there.

@matthijskooijman matthijskooijman transferred this issue from arduino/Arduino Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants