Hi Saurav, You can follow the steps here: https://nuttx.apache.org/docs/latest/contributing/index.html
After running the checkpatch.sh and confirming everything is fine, you can create a commit. Basically your commit should be like this: ------------------------------------------------------------------------------ fs/vfat: Fix typo in the macro DIRSEC_BYTENDX Some comments explaining what the commit is supposed to do! Signed-off-by: Saurav Pal <resyfer....@gmail.com> ------------------------------------------------------------------------------ Best Regards, Alan On Fri, Feb 2, 2024 at 5:14 PM Saurav Pal <resyfer....@gmail.com> wrote: > 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 > > > > > >
