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

Fix stackoverflow due to recursion in vfs_spiffs_readdir_r #2212

Merged
merged 1 commit into from
Jul 28, 2018
Merged

Fix stackoverflow due to recursion in vfs_spiffs_readdir_r #2212

merged 1 commit into from
Jul 28, 2018

Conversation

konstk1
Copy link
Contributor

@konstk1 konstk1 commented Jul 20, 2018

Current implementation of vfs_spiffs_readdir_r() has an issue with recursion. In the case where you have N consecutive files in /folder1/ and you're reading /folder2/, vfs_spiffs_readdir_r() gets recursively called N times. When N get sufficiently large (in my case around 35), stack overflows.

I updated vfs_spiffs_readdir_r() to use a do/while loop instead.

NOTE: I did not extensively test this beyond my specific use case.

@CLAassistant
Copy link

CLAassistant commented Jul 20, 2018

CLA assistant check
All committers have signed the CLA.

@Ritesh236
Copy link

Is there any plan to merge this change into master or current running branch?

@davctv
Copy link

davctv commented Jul 22, 2018

Hello ritesh,
I have asked to esp_igrr on esp32.com forum the time schedule. Did you tested the PR branch? I Will do soon to check if it works...

@davctv
Copy link

davctv commented Jul 23, 2018

I have tested the PR and to me it seems working. thanks

@igrr igrr added the Status: Pending blocked by some other factor label Jul 23, 2018
@igrr
Copy link
Member

igrr commented Jul 23, 2018

Thanks for the PR @kostyan5, this should be merged soon.

@igrr igrr merged commit 3486b51 into espressif:master Jul 28, 2018
@igrr igrr removed the Status: Pending blocked by some other factor label Aug 9, 2018
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

Successfully merging this pull request may close these issues.

None yet

5 participants