Hi Alan, Yeah I'll add an issue in the repo, and put a PR for fixing this. A quick question...is there any standard on commit names, or should I just use my own like "FIX: VFAT macro Issue#123"?
About why side effects have not been seen...like Lwazi explained, macro DIRSEC_BYTENDX(f, i) is always used as DIRSEC_BYTENDX(fs, inode), and so, in its expansion, DIRSEC_NDXMASK(f) should become DIRSEC_NDXMASK(fs), and that weirdly coincides with what the typo is, and so, it weirdly works how it was intended to and does not give any errors. Since it is a macro, even language servers (as far as the ones I know) do not visually show their "unused parameters" warning like in the case proper functions. Regards, Saurav On Sat, Feb 3, 2024 at 1:11 AM Alan C. Assis <acas...@gmail.com> wrote: > Hi Saurav, > > I think you found a BUG! > > Please report it at https://github.com/apache/nuttx/issues to keep a track > of it and when you we submit a PR it could be closed automatically (since > you link it at your PR). > > In fact looking both macros the f to fs mistake becomes clear: > > #define DIRSEC_NDXMASK(f) (((f)->fs_hwsectorsize - 1) >> 5) > > #define DIRSEC_BYTENDX(f,i) (((i) & DIRSEC_NDXMASK(fs)) << 5) > > I'm curious to know what this mistake could cause to VFAT on NuttX? > > Why haven't we seen any side effects in products using NuttX with VFAT? > > Best Regards, > > Alan > > On Fri, Feb 2, 2024 at 1:55 PM Saurav Pal <resyfer....@gmail.com> wrote: > > > Hi Alan, > > > > Thank you for looking at our code base and planning to add Documentation, > > > that is really important! > > > > > > NuttX has a long history but our Documentation is still lagging behind, > > so > > > your work will be very beneficial for our community. > > > > > > > I'll try my best to contribute some documentation for the various file > > systems I look into, and I, in turn, hope they are informative, and more > > importantly, correct, as I'm pretty new to the VFS layer of NuttX. > > > > > > > BTW, why do you think it is wrong, could you please share your > thoughts? > > > > > > > The problem according to me is on L264...where it's defined as: > > > > #define DIRSEC_BYTENDX(*f*,i) (((i) & DIRSEC_NDXMASK(*fs*)) << 5) > > > > I can't seem to find anything defined as *fs *in the namespace at that > > point (used in the fat_findsfnentry > > < > https://github.com/apache/nuttx/blob/master/fs/fat/fs_fat32dirent.c#L1137 > > >function), > > and I was wondering if it was a simple typo or a mistake on my part as > it's > > code that's quite old, which usually don't have such mistakes due to > > extensive testing. > > > > Best Regards, > > Saurav > > >
