compuphase / minIni

A small and portable INI file library with read/write support
http://www.compuphase.com/minini.htm
Other
382 stars 114 forks source link

Glue for SPIFFS #9

Open timothyjohncarney opened 6 years ago

timothyjohncarney commented 6 years ago

I have adapted this project for use with SPIFFS.

https://github.com/pellepl/spiffs

In order to do so, I needed to make a few changes.

  1. Statically allocate the LocalBuffers to reduce call stack usage.
  2. Remove case-insensitive search as it is not C99 compatible.
  3. Add a wrapper function to SPIFFS file read API to behave like fgets.

I am willing to share my changes, but I am unsure if these changes are of interest. I believe the first change will break the reentrancy. Some locking mechanism is necessary to guarantee reentrancy after moving the buffers off of the call stack. Furthermore, removing support for case-insensitive search seems like a step in the wrong direction.

Please advise and thank you for your work.

-Tim

compuphase commented 6 years ago

Hello Tim,

I am certainly interested in the SPIFFS wrapper functions. Re-entrancy is only an issue when using a multitasking kernel. I think that people using a multitasking kernel, will also use a microcontroller with sufficient RAM for all task stacks. Which functions are not C99 compatible? An easy fix would be to add portable implementations of those functions in the minIni source (conditionally compiled, so that we use the library functions when available).

Regards, Thiadmer Riemersma

On Tue, Jun 5, 2018 at 6:49 PM, Tim notifications@github.com wrote:

I have adapted this project for use with SPIFFS.

https://github.com/pellepl/spiffs

In order to do so, I needed to make a few changes.

  1. Statically allocate the LocalBuffers to reduce call stack usage.
  2. Remove case-insensitive search as it is not C99 compatible.
  3. Add a wrapper function to SPIFFS file read API to behave like fgets.

I am willing to share my changes, but I am unsure if these changes are of interest. I believe the first change will break the reentrancy. Some locking mechanism is necessary to guarantee reentrancy after moving the buffers off of the call stack. Furthermore, removing support for case-insensitive search seems like a step in the wrong direction.

Please advise and thank you for your work.

-Tim

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/compuphase/minIni/issues/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AK7TQ4XerrtHvDY8Nc3f-ChPJ0MPhww1ks5t5rajgaJpZM4UbMBp .

timothyjohncarney commented 6 years ago

Thiadmer,

The string-insensitive comparison was not C99 compatible, but I did find PORTABLE_STRNICMP since my last message, so I do not think that is a concern.

I am using FreeRTOS, but I still have limited RAM so I am unable to support the large buffers on the call stack. I have declared the buffers as static and that allows me to run without allocating as much RAM to the call stack of each task I wish to use with minIni.

-Tim

timothyjohncarney commented 6 years ago

Thiadmer,

I am happy to push the changes I made to a feature branch and create a pull request. Would you please grant me push access to the repository so that I may contribute those files?

Thanks, Tim

compuphase commented 6 years ago

Hello Tim,

I have added the branch SPIFFS to minIni. I have also added you as a collaborator to the repository. I think it means that you have access to all projects (and all branches). I did not find out how to restrict the push access to just a specific branch. I think you need an "organization account" to do that.

Regards, Thiadmer Riemersma

On Fri, Jun 8, 2018 at 3:48 PM, Tim notifications@github.com wrote:

Thiadmer,

I am happy to push the changes I made to a feature branch and create a pull request. Would you please grant me push access to the repository so that I may contribute those files?

Thanks, Tim

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/compuphase/minIni/issues/9#issuecomment-395766090, or mute the thread https://github.com/notifications/unsubscribe-auth/AK7TQ6GLw2vqCfumMAXoyTgrOKfJwi1Xks5t6oCmgaJpZM4UbMBp .

timothyjohncarney commented 6 years ago

Thiadmer,

I have pushed my changes for review.

Thank you, Tim