Patchwork [U-Boot] FAT: make FAT compile without VFAT

login
register
mail settings
Submitter Richard Genoud
Date Dec. 4, 2012, 1:04 p.m.
Message ID <1354626246-9613-1-git-send-email-richard.genoud@gmail.com>
Download mbox | patch
Permalink /patch/203616/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Richard Genoud - Dec. 4, 2012, 1:04 p.m.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 fs/fat/fat.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Marek Vasut - Dec. 4, 2012, 2:24 p.m.
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
Richard Genoud - Dec. 4, 2012, 3:32 p.m.
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
Stefano Babic - Dec. 4, 2012, 4:16 p.m.
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
Richard Genoud - Dec. 13, 2012, 10:47 a.m.
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(-)
Marek Vasut - Dec. 13, 2012, 12:45 p.m.
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
Richard Genoud - Dec. 13, 2012, 12:57 p.m.
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 !

Patch

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 &&