Fixed esp_core_dump_image_erase() for flash encryption with 16byte long write buffers (IDFGH-5173) #6949
Conversation
This file contains 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
github-actions
bot
changed the title
|
@0xFEEDC0DE64 Thanks for your contribution. |
|
Is this fix already merged? or is there anything I can do to help? |
|
Hello @0xFEEDC0DE64, Sorry for the delay of response, thank you for your contribution once again 😄 ! |
o-marshmallow
suggested changes
Jun 1, 2021
| @@ -484,9 +486,21 @@ esp_err_t esp_core_dump_image_erase(void) | |||
| return err; | |||
| } | |||
|
|
|||
| // helper to create (multiple of) 16 byte long write buffers | |||
There was a problem hiding this comment.
There is a simpler way to do that I would prefer, it's to add at the beginning of this function the following snippet:
/* If flash is encrypted, we can only write blocks of 16 bytes. */
uint32_t helper[4] = { BLANK_COREDUMP_SIZE };
You can add the static assert right after (same as now, sizeof(helper) will return 16).
| helper.size = BLANK_COREDUMP_SIZE; | ||
|
|
||
| // fill the remaining bytes | ||
| memset(&helper.buf, '\0', sizeof(helper.buf)); |
There was a problem hiding this comment.
If you follow my comment above, no need to memset the memory
| @@ -12,6 +12,8 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
| #include <string.h> | |||
| #include <assert.h> | |||
| #include "esp_core_dump.h" | |||
There was a problem hiding this comment.
Was these includes strictly required? _Static_assert can be used without them
espressif-bot
added
Status: In Progress
espressif-bot
added
Status: Done
espressif-bot
pushed a commit
that referenced
this pull request
Oct 12, 2021
Sign up for free
to 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.
esp_partition_write() kept failing in esp_core_dump_image_erase() when I enabled flash encryption, as the write function then requires multiple of 16 byte long write buffers.
I changed the erase method so it always writes 16 bytes and the blank coredump size at the beginning.
Also I added a compile time static assert to make sure the struct has a valid size.