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

Inconsistent use of offset in ESP.rtcUserMemoryRead and ESP.rtcUserMemoryWrite in Esp.cpp #2875

Closed
hassebjork opened this issue Jan 17, 2017 · 1 comment
Assignees
Milestone

Comments

@hassebjork
Copy link

The variable offset in ESP.rtcUserMemoryRead and ESP.rtcUserMemoryWrite is used inconsistently. Either it should be in bytes or block of 4 bytes. (Probably the latter)

The corresponding functions in (system_rtc_mem_read and system_rtc_mem_write) "user_interface.h" uses a multiple of 4 bytes for src_addr and dst_addr. The total rtc memory amount is 768 bytes, the first 256 bytes reserved for system use and 512 bytes available to the user!

In the "Esp.cpp" offset is used as a multiple of 4 bytes in these lines:
return system_rtc_mem_read(64 + offset, data, size);
return system_rtc_mem_write(64 + offset, data, size);
(64 blocks being the reserved memory, 256 bytes / 4)

A few lines above these, a test uses offset as a single byte:
if (size + offset > 512)
return false;

If offset represents blocks of 4 bytes, it should read
if (size + offset*4 > 512)
or else
return system_rtc_mem_read(64 + offset/4, data, size);
return system_rtc_mem_write(64 + offset/4, data, size);
(The latter being rather stupid)

By the way, does offset need to be uint32_t when it is converted to uint8 in the system calls?

@devyte
Copy link
Collaborator

devyte commented Oct 4, 2017

@hassebjork you are correct, the check if (size + offset > 512) needs to be fixed in both functions. Issue is still valid in latest git.
Care to make a PR?

@devyte devyte added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Oct 4, 2017
@devyte devyte self-assigned this Feb 8, 2018
@devyte devyte added this to the 2.5.0 milestone Feb 8, 2018
@devyte devyte added staged-for-release and removed waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Nov 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants