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

Bugs in sensorcal #16

Open
hpmax opened this issue May 31, 2020 · 0 comments
Open

Bugs in sensorcal #16

hpmax opened this issue May 31, 2020 · 0 comments

Comments

@hpmax
Copy link
Contributor

hpmax commented May 31, 2020

There are multiple bugs in sensorcal related to the serial number.

Line 90 claims the the serial number is 6 characters -- which matches the data structure in 24c16.h

Line 127 does strcpy (data.serial,"000000") --- This is an issue because strcpy will terminate with a 0x00 at the end, which means it's actually loading 7 bytes into a 6 byte array likely messing up the first byte of zero_offset. Fortunately, this is not a serious problem since line 128 initializes zero_offset to 0.

Lines 192-196 Load 7 bytes into data.serial, again... This is probably not a big deal because the data structure in the RAM and the EEPROM presumably match, which means you're corrupting the 1st byte of zero_offset with itself.

Line 197 adds a "\n" to the end of it, corrupting the second byte of zero_offset. This makes no sense since a string should be terminated by a 0x00 not a \n. Unlike line 127, after executing line 197, line 201 is executed which writes the now corrupted data back into the EEPROM.

I propose to add a local variable char s[7] to deal with lines 192-198:
Line 127: changes from strcpy("000000",data.serial) to memstr(data.serial,'0',6)
Line 192: changes from i<7 to i<6
Line 194: changes from data.serial[i]=*optarg to data.serial[i]=s[i]=*optarg
Line 197: changes from data.serial[7]='/n' to s[6]=0;
Line 198: changes from data.serial to s

I'll fix these after I get my voltage and temperature pulls merged.

There are a few additional issues where we are ignoring return values which we probably shouldn't throughout sensord, but I wouldn't necessarily call those bugs and are probably lower priority.

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

1 participant