| Message ID | 1354626246-9613-1-git-send-email-richard.genoud@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Richard Genoud, Please provide a reasonable commit message. Why is this needed? I really dislike the amount of added #ifdefs :( [...] Best regards, Marek Vasut
2012/12/4 Marek Vasut <marex@denx.de>: > Dear Richard Genoud, > > Please provide a reasonable commit message. Why is this needed? hi, You're right, my commit message is nearly inexistent.. sorry. This is needed in the case where we don't want the VFAT support (for whatever reason). in this case, we #undef CONFIG_SUPPORT_VFAT (in fat.h) and it breaks at : csum = mkcksum(dentptr->name, dentptr->ext); > I really dislike the amount of added #ifdefs :( > [...] yes, I wasn't very comfortable with that either, I'll come with something cleaner. Best regards, Richard. > > Best regards, > Marek Vasut
On 04/12/2012 14:04, Richard Genoud wrote: > Signed-off-by: Richard Genoud <richard.genoud@gmail.com> > --- > fs/fat/fat.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/fs/fat/fat.c b/fs/fat/fat.c > index 393c378..defdd74 100644 > --- a/fs/fat/fat.c > +++ b/fs/fat/fat.c > @@ -589,7 +589,9 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect, > char *filename, dir_entry *retdent, > int dols) > { > +#ifdef CONFIG_SUPPORT_VFAT > __u16 prevcksum = 0xffff; > +#endif You can declare the variable __maybe_unused without adding the #ifdef > __u32 curclust = START(retdent); > int files = 0, dirs = 0; > > @@ -828,7 +830,9 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, > fsdata datablock; > fsdata *mydata = &datablock; > dir_entry *dentptr = NULL; > +#ifdef CONFIG_SUPPORT_VFAT > __u16 prevcksum = 0xffff; > +#endif Ditto > +#ifdef CONFIG_SUPPORT_VFAT > csum = mkcksum(dentptr->name, dentptr->ext); > +#endif I think this is the only place where #ifdef is necessary Best regards, Stefano Babic
Hi, Instead of adding some more ugly #ifdefs, I came with another approach. It makes the code easier to read, and correct the compilation error when VFAT wasn't enabled. Best regards, Richard Genoud (2): FAT: remove ifdefs to make the code more readable FAT: use toupper/tolower instead of recoding them fs/fat/fat.c | 58 +++++++++++++++++++++++++++------------------------ fs/fat/fat_write.c | 14 ++++-------- include/fat.h | 3 -- 3 files changed, 36 insertions(+), 39 deletions(-)
Dear Richard Genoud, > Hi, > > Instead of adding some more ugly #ifdefs, I came with another approach. > It makes the code easier to read, and correct the compilation error > when VFAT wasn't enabled. I feel bad for the code when you say it's FAT so explicitly ... it certainly wishes it was SLIM, but it can't ;-) Best regards, Marek Vasut
2012/12/13 Marek Vasut <marex@denx.de>: > Dear Richard Genoud, > > I feel bad for the code when you say it's FAT so explicitly ... it certainly > wishes it was SLIM, but it can't ;-) > :) I didn't write that on purpose... But it's definitely a good war cry !
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 393c378..defdd74 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -589,7 +589,9 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect, char *filename, dir_entry *retdent, int dols) { +#ifdef CONFIG_SUPPORT_VFAT __u16 prevcksum = 0xffff; +#endif __u32 curclust = START(retdent); int files = 0, dirs = 0; @@ -828,7 +830,9 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, fsdata datablock; fsdata *mydata = &datablock; dir_entry *dentptr = NULL; +#ifdef CONFIG_SUPPORT_VFAT __u16 prevcksum = 0xffff; +#endif char *subname = ""; __u32 cursect; int idx, isdir = 0; @@ -944,7 +948,9 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, for (i = 0; i < DIRENTSPERBLOCK; i++) { char s_name[14], l_name[VFAT_MAXLEN_BYTES]; +#ifdef CONFIG_SUPPORT_VFAT __u8 csum; +#endif l_name[0] = '\0'; if (dentptr->name[0] == DELETED_FLAG) { @@ -952,7 +958,9 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, continue; } +#ifdef CONFIG_SUPPORT_VFAT csum = mkcksum(dentptr->name, dentptr->ext); +#endif if (dentptr->attr & ATTR_VOLUME) { #ifdef CONFIG_SUPPORT_VFAT if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
Signed-off-by: Richard Genoud <richard.genoud@gmail.com> --- fs/fat/fat.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)