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

Set value #743

Merged
merged 2 commits into from Sep 26, 2018
Merged

Set value #743

merged 2 commits into from Sep 26, 2018

Conversation

dontsovcmc
Copy link
Contributor

@dontsovcmc dontsovcmc commented Sep 24, 2018

Firstly, I correct your error:
if(_length < length){ to if(_length < deflength){
but it is dangerous to use strlen for default value cause overflow.
I remove you code to increate length.

Secondly,
a. memset
b. need not _value[_length] = '\0'; case already memset with length+1

  1. Now init() and getValue() are symmetrical for parameter length: you set and get with same limit.
  2. init() parameter is safe: you can use EEPROM buffer without null terminator.

@tablatronix
Copy link
Collaborator

geez how the hell did I do that.. thanks lol
yeah memset is probably ideal here

@tablatronix
Copy link
Collaborator

I think you are correct that the length should not change if defvalue is longer, but you removed the checking, I would like to keep that in to throw debug errors. That is why I had the comment in there.

@tablatronix
Copy link
Collaborator

strlen is probably not multibyte safe though? So not sure what to do here, just truncate def value, any ideas

@tablatronix tablatronix added the bug Validated BUG label Sep 24, 2018
@tablatronix tablatronix added this to the dev milestone Sep 24, 2018
@dontsovcmc
Copy link
Contributor Author

sorry, i mean dangerous to allocate huge strlen return value

@tablatronix tablatronix merged commit 6dbc001 into tzapu:development Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Validated BUG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants