You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/12/21 05:19:39 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #2572: spiffs fixes
yamt opened a new pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572
## Summary
Make spiffs work on images built with the following tools:
* https://github.com/igrr/mkspiffs
* spiffsgen.py from ESP-IDF
## Impact
this breaks on-flash compatibility with the previous versions
## Testing
tested locally with esp32 devkitc and the above mentioned tools
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546521690
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -330,7 +330,14 @@ static int spiffs_readdir_callback(FAR struct spiffs_s *fs,
DEBUGASSERT(dir != NULL);
entryp = &dir->fd_dir;
- strncpy(entryp->d_name, (FAR char *)objhdr.name, NAME_MAX + 1);
+ /* Skip the leading '/'. */
+
+ if (objhdr.name[0] != '/')
Review comment:
do you have specific reasons to make this that smart?
generally i don't think it's a good idea to put this kind of smart things unless absolutely necessary.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546557988
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
> > the structure with this PR should be compatible with:
> >
> > * https://github.com/pellepl/spiffs (the version used by mkspiffs. i dunno if 0.4.0 changed it or not)
> > * https://github.com/igrr/mkspiffs
> > * ESP-IDF spiffsgen.py
>
> If so, we need change the NuttX version to confirm the standard as soon as possible.
it's what this PR does.
>
> > > And these patches also don't need anymore:
> > > fs/spiffs/Kconfig: Update the description of SPIFFS_NAME_MAX
> > > spiffs: Fix statfs f_namelen
> >
> >
> > i'm not sure about this point.
> > do you prefer to keep SPIFFS_NAME_MAX not including the leading '/'?
> > in that case, a user need to specify SPIFFS_NAME_MAX+1 for tools like mkspiffs.
>
> From your patch, '/' is always inserted before the relpath. If spiffs_find_objhdr_pgndx can handle relpath without the leading '/' correctly. Then we don't need reserve one space for '/'?
regardless of what nuttx do, those tools take a length including the leading slash.
in the latest push, i made it conditional for people who prefer compatibility with older nuttx.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546520606
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -330,7 +330,14 @@ static int spiffs_readdir_callback(FAR struct spiffs_s *fs,
DEBUGASSERT(dir != NULL);
entryp = &dir->fd_dir;
- strncpy(entryp->d_name, (FAR char *)objhdr.name, NAME_MAX + 1);
+ /* Skip the leading '/'. */
+
+ if (objhdr.name[0] != '/')
Review comment:
how about:
if (objhdr.name[0] != '/')
{
strncpy(entryp->d_name, (FAR char *)objhdr.name , NAME_MAX + 1);
}
else
{
strncpy(entryp->d_name, (FAR char *)objhdr.name + 1, NAME_MAX + 1);
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748822930
> > This is the test config in the sim
> > https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Fsim%2Fsim%2Fsim%2Fconfigs%2Fspiffs%2Fdefconfig
>
> thank you. i run it with and without this PR. the output was identical.
btw, i couldn't find where the spiffs filesystem is initialized for the config.
is the test assuming that the initial state of RAM MTD is a valid spiffs?
is it the right thing to do?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-755167281
> Look good to me now. But @yamt could you reorg your pathset to more reasonable?
>
> 1. Merge "fs/spiffs/Kconfig: Add SPIFFS_COMPAT_OLD_NUTTX" and "spiffs_pgobj_ndxheader_s: Add a missing alignment"
done
> 2. Merge "spiffs: Prefix filenames with '/' as other implmenetations do" and "spiffs: Suggest SPIFFS_LEADING_SLASH=n for better compatibility"
done
> 3. Merge "spiffs: Rename the upsteam readme to README-spiffs.md" and "spiffs: Document how to generate images"
i don't want to do this because it makes a diff difficult to read.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546546091
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
> the structure with this PR should be compatible with:
>
> * https://github.com/pellepl/spiffs (the version used by mkspiffs. i dunno if 0.4.0 changed it or not)
> * https://github.com/igrr/mkspiffs
> * ESP-IDF spiffsgen.py
If so, we need change the NuttX version to confirm the standard as soon as possible.
> > And these patches also don't need anymore:
> > fs/spiffs/Kconfig: Update the description of SPIFFS_NAME_MAX
> > spiffs: Fix statfs f_namelen
>
> i'm not sure about this point.
> do you prefer to keep SPIFFS_NAME_MAX not including the leading '/'?
> in that case, a user need to specify SPIFFS_NAME_MAX+1 for tools like mkspiffs.
From your patch, '/' is always inserted before the relpath. If spiffs_find_objhdr_pgndx can handle relpath without the leading '/' correctly. Then we don't need reserve one space for '/'?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546553786
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-749263509
after reading https://github.com/pellepl/spiffs code a bit, i feel the leading slash might not be essential in the on-flash structure.
it's probably just how mkspiffs and spiffsgen.py generate an image from a directory.
i guess nothing prevents a spiffs-using software from using SPIFFS_open (the api in the original spiffs)
with a name without slashes.
on the other hand, the alignement things seems essential.
i guess it makes more sense to have two separate Kconfig for them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748772892
> This is the test config in the sim
>
> https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Fsim%2Fsim%2Fsim%2Fconfigs%2Fspiffs%2Fdefconfig
thank you. i run it with and without this PR. the output was identical.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546530139
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
> one place? you mean three?
>
we need change at least spiffs_open, spiffs_unlink, spiffs_rename and spiffs_stat with the current patch. On the other hand, if we enhance spiffs_find_objhdr_pgndx to support the name without leading '\', all spiffs_xxx don't need change.
And these patches also don't need anymore:
fs/spiffs/Kconfig: Update the description of SPIFFS_NAME_MAX
spiffs: Fix statfs f_namelen
> i can change it if you feel strongly.
> but i guess it isn't the biggest issue for this PR.
> the incompatibility with older nuttx is.
Yes, the padding added inside spiffs_pgobj_ndxheader_s is more critical than others. We need reference the offical spiffs to see how they define this structure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-753765765
does any of you still have any concerns?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546519955
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
can we enhance spiffs_find_objhdr_pgndx and spiffs_fobj_create to handle with or without the leading '/' correclty?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546540073
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
> And these patches also don't need anymore:
> fs/spiffs/Kconfig: Update the description of SPIFFS_NAME_MAX
> spiffs: Fix statfs f_namelen
i'm not sure about this point.
do you prefer to keep SPIFFS_NAME_MAX not including the leading '/'?
in that case, a user need to specify SPIFFS_NAME_MAX+1 for tools like mkspiffs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748769018
This is the test config in the sim
https://www.github.com/apache/incubator-nuttx/tree/master/boards%2Fsim%2Fsim%2Fsim%2Fconfigs%2Fspiffs%2Fdefconfig
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546524978
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
one place? you mean three?
i can change it if you feel strongly.
but i guess it isn't the biggest issue for this PR.
the incompatibility with older nuttx is.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748888489
@btashton and @patacongo should we keep the compatibility(mistake) layer?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-749284032
> i guess it makes more sense to have two separate Kconfig for them.
i separated them
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748767874
> Have you run this against the test tools in the sim? Last time there were some changes that broke the tests.
which test tools?
> Also we need to figure out what to do about backwards compatibility. I don't think that breaking people who are upgrading is going to be acceptable.
is anyone using spiffs?
if yes, i want to know how they are using it.
the current code doesn't seem to work with mkspiffs or spiffsgen. (that's why i made this PR)
but i suspect there is a tool i'm not aware of and it's compatible with the current nuttx implementation.
> Can you describe what exactly changed in the format? Maybe we can have a compatibility config flag.
* the leading '/' in the object names
* the alignment in spiffs_pgobj_ndxheader_s
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546527192
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -330,7 +330,14 @@ static int spiffs_readdir_callback(FAR struct spiffs_s *fs,
DEBUGASSERT(dir != NULL);
entryp = &dir->fd_dir;
- strncpy(entryp->d_name, (FAR char *)objhdr.name, NAME_MAX + 1);
+ /* Skip the leading '/'. */
+
+ if (objhdr.name[0] != '/')
Review comment:
Sure, it's fine if you confirm the lead '/' always exist.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-755176697
Ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748822205
i introduced a Kconfig for the compatibility with older nuttx versions.
i haven't tested it as i don't know how to create images for them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546521428
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
The change add local variable and memcpy which increase the stack usage and reduce the performance. And every caller of spiffs_find_objhdr_pgndx/spiffs_fobj_create need to do the same thing repeatly. If we handle the leading '/' inside spiffs_find_objhdr_pgndx/spiffs_fobj_create, we can avoid the local variable and only change one place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546536247
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
the structure with this PR should be compatible with:
* https://github.com/pellepl/spiffs (the version used by mkspiffs. i dunno if 0.4.0 changed it or not)
* https://github.com/igrr/mkspiffs
* ESP-IDF spiffsgen.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#discussion_r546520590
##########
File path: fs/spiffs/src/spiffs_vfs.c
##########
@@ -408,7 +416,8 @@ static int spiffs_open(FAR struct file *filep, FAR const char *relpath,
/* Check of the file object already exists */
- ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)relpath, &pgndx);
+ ret = spiffs_find_objhdr_pgndx(fs, (FAR const uint8_t *)spiffs_relpath,
Review comment:
if you mean depends on a config, we can. but i feel spiffs_vfs.c is a better place to handle nuttx-specific glitches like this.
if you mean a runtime detection, i feel it's overkill.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #2572: spiffs fixes
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2572:
URL: https://github.com/apache/incubator-nuttx/pull/2572#issuecomment-748765398
Have you run this against the test tools in the sim? Last time there were some changes that broke the tests.
Also we need to figure out what to do about backwards compatibility. I don't think that breaking people who are upgrading is going to be acceptable.
Can you describe what exactly changed in the format? Maybe we can have a compatibility config flag.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org