Patchwork [U-Boot,1/2] FAT: remove ifdefs to make the code more readable

login
register
mail settings
Submitter Richard Genoud
Date Dec. 13, 2012, 10:47 a.m.
Message ID <1355395656-5838-2-git-send-email-richard.genoud@gmail.com>
Download mbox | patch
Permalink /patch/205799/
State Superseded
Headers show

Comments

Richard Genoud - Dec. 13, 2012, 10:47 a.m.
ifdefs in the code are making it harder to read.
The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
(the code is discarded by the compiler and linker instead of the
preprocessor.)

and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
defined.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 fs/fat/fat.c       |   55 +++++++++++++++++++++++++++------------------------
 fs/fat/fat_write.c |   11 ++-------
 2 files changed, 32 insertions(+), 34 deletions(-)
Marek Vasut - Dec. 13, 2012, 12:46 p.m.
Dear Richard Genoud,

> ifdefs in the code are making it harder to read.
> The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
> (the code is discarded by the compiler and linker instead of the
> preprocessor.)
> 
> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
> defined.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  fs/fat/fat.c       |   55
> +++++++++++++++++++++++++++------------------------ fs/fat/fat_write.c |  
> 11 ++-------
>  2 files changed, 32 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 393c378..c79e3e3 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -34,6 +34,12 @@
>  #include <malloc.h>
>  #include <linux/compiler.h>
> 
> +#ifdef CONFIG_SUPPORT_VFAT
> +#define VFAT_ENABLED 1
> +#else
> +#define VFAT_ENABLED 0
> +#endif
[...]

Make it static const int maybe ?

Best regards,
Marek Vasut
Richard Genoud - Dec. 13, 2012, 1 p.m.
2012/12/13 Marek Vasut <marex@denx.de>:
> Dear Richard Genoud,
>
>> ifdefs in the code are making it harder to read.
>> The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
>> (the code is discarded by the compiler and linker instead of the
>> preprocessor.)
>>
>> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
>> defined.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  fs/fat/fat.c       |   55
>> +++++++++++++++++++++++++++------------------------ fs/fat/fat_write.c |
>> 11 ++-------
>>  2 files changed, 32 insertions(+), 34 deletions(-)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index 393c378..c79e3e3 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -34,6 +34,12 @@
>>  #include <malloc.h>
>>  #include <linux/compiler.h>
>>
>> +#ifdef CONFIG_SUPPORT_VFAT
>> +#define VFAT_ENABLED 1
>> +#else
>> +#define VFAT_ENABLED 0
>> +#endif
> [...]
>
> Make it static const int maybe ?
I hesitate to make them static const, I can't figure why it's better
to use static const variable instead of defines.
Could you enlighten me ?

Best regards,
Richard.
Tom Rini - Dec. 13, 2012, 1:09 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/13/12 05:47, Richard Genoud wrote:
> ifdefs in the code are making it harder to read. The use of simple
> if(VFAT_ENABLED) makes no more code and is cleaner. (the code is
> discarded by the compiler and linker instead of the preprocessor.)
> 
> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is
> not defined.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com> --- 
> fs/fat/fat.c       |   55
> +++++++++++++++++++++++++++------------------------ 
> fs/fat/fat_write.c |   11 ++------- 2 files changed, 32
> insertions(+), 34 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 393c378..c79e3e3
> 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -34,6 +34,12 @@ 
> #include <malloc.h> #include <linux/compiler.h>
> 
> +#ifdef CONFIG_SUPPORT_VFAT +#define VFAT_ENABLED 1 +#else +#define
> VFAT_ENABLED 0 +#endif + /* * Convert a string to lowercase. */ @@
> -441,7 +447,6 @@ getit: } while (1); }
> 
> -#ifdef CONFIG_SUPPORT_VFAT /* * Extract the file name information
> from 'slotptr' into 'l_name', * starting at l_name[*idx]. @@ -576,7
> +581,6 @@ static __u8 mkcksum(const char name[8], const char
> ext[3])
> 
> return ret; } -#endif	/* CONFIG_SUPPORT_VFAT */
> 
> /* * Get the directory entry associated with 'filename' from the
> directory @@ -617,8 +621,8 @@ static dir_entry
> *get_dentfromdir(fsdata *mydata, int startsect, continue; } if
> ((dentptr->attr & ATTR_VOLUME)) { -#ifdef CONFIG_SUPPORT_VFAT -
> if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT && +				if
> (VFAT_ENABLED && +				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT
> && (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { prevcksum =
> ((dir_slot *)dentptr)->alias_checksum; get_vfatname(mydata,
> curclust, @@ -658,9 +662,7 @@ static dir_entry
> *get_dentfromdir(fsdata *mydata, int startsect, continue; } 
> debug("vfatname: |%s|\n", l_name); -				} else -#endif -				{ +
> } else { /* Volume label or VFAT entry */ dentptr++; continue; @@
> -674,14 +676,15 @@ static dir_entry *get_dentfromdir(fsdata
> *mydata, int startsect, debug("Dentname == NULL - %d\n", i); return
> NULL; } -#ifdef CONFIG_SUPPORT_VFAT -			__u8 csum =
> mkcksum(dentptr->name, dentptr->ext); -			if (dols && csum ==
> prevcksum) { -				prevcksum = 0xffff; -				dentptr++; -
> continue; +			if (VFAT_ENABLED) { +				__u8 csum =
> mkcksum(dentptr->name, dentptr->ext); +				if (dols && csum ==
> prevcksum) { +					prevcksum = 0xffff; +					dentptr++; +
> continue; +				} } -#endif + get_name(dentptr, s_name); if (dols)
> { int isdir = (dentptr->attr & ATTR_DIR); @@ -884,9 +887,9 @@
> do_fat_read_at(const char *filename, unsigned long pos, void
> *buffer, return -1; }
> 
> -#ifdef CONFIG_SUPPORT_VFAT -	debug("VFAT Support enabled\n"); 
> -#endif +	if (VFAT_ENABLED) +		debug("VFAT Support enabled\n"); + 
> debug("FAT%d, fat_sect: %d, fatlength: %d\n", mydata->fatsize,
> mydata->fat_sect, mydata->fatlength); debug("Rootdir begins at
> cluster: %d, sector: %d, offset: %x\n" @@ -952,10 +955,12 @@
> do_fat_read_at(const char *filename, unsigned long pos, void
> *buffer, continue; }
> 
> -			csum = mkcksum(dentptr->name, dentptr->ext); +			if
> (VFAT_ENABLED) +				csum = mkcksum(dentptr->name, dentptr->ext); + 
> if (dentptr->attr & ATTR_VOLUME) { -#ifdef CONFIG_SUPPORT_VFAT -
> if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT && +				if
> (VFAT_ENABLED && +				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT
> && (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { prevcksum = 
> ((dir_slot *)dentptr)->alias_checksum; @@ -999,9 +1004,7 @@
> do_fat_read_at(const char *filename, unsigned long pos, void
> *buffer, } debug("Rootvfatname: |%s|\n", l_name); -				} else 
> -#endif -				{ +				} else { /* Volume label or VFAT entry */ 
> dentptr++; continue; @@ -1015,13 +1018,13 @@ do_fat_read_at(const
> char *filename, unsigned long pos, void *buffer, } goto exit; } 
> -#ifdef CONFIG_SUPPORT_VFAT -			else if (dols == LS_ROOT && csum ==
> prevcksum) { +			else if (VFAT_ENABLED && +				 dols == LS_ROOT &&
> csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } 
> -#endif + get_name(dentptr, s_name);
> 
> if (dols == LS_ROOT) { diff --git a/fs/fat/fat_write.c
> b/fs/fat/fat_write.c index 4a1bda0..ed3eaa0 100644 ---
> a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -248,7 +248,6 @@
> static __u32 get_fatent_value(fsdata *mydata, __u32 entry) return
> ret; }
> 
> -#ifdef CONFIG_SUPPORT_VFAT /* * Set the file name information from
> 'name' into 'slotptr', */ @@ -468,8 +467,6 @@
> get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster, 
> return 0; }
> 
> -#endif

Note that we don't use --gc-sections on all archs so I'm not sure we
discard the unused VFAT functions on say ARM.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQydOVAAoJENk4IS6UOR1WDYQP/2P6ywBeeE/qnx9C13qJnCcr
wXG1IJWEpjgAEOViQZ77kE5Knn9ltiuXKj6EYc7e3oY4S2mBREkG/sHNmG+e+9/0
WHuZJoaiLPaia9ztL/d8BCyVPkSbJ88iyA20TgefR94j7SqITiVT+3yDA4XEB3bW
C8rXZdmYzGjLzMBPI458uxNmIKSeHVfUZnDdWdA0rIOK8t6QN+OKLJRMezOYwx8e
jM8Td4hDmJ20EGfFTpcO2spdUMCqi6FfO6bXJHeFHeUqGXqz482xyIxiZGpETut9
tFOVegHIpbKf0eBoMvs+3ajHprAsMGFlAqv0YSxh4w04ZIXUBT+DibjGiQtf/Xjl
HDvAkBGEbQjljZaQDYlaAfNWFctU7lkapujiMuOKjv04DvClp0AgKEvOjAUW/P2N
ERIQzc67dUdomhmdl0sl+yFYJDB4zjT1VnlYXGx/yz4KLIiD5t+gaQAyq3pGwYjH
jZ3q5gAprYNR0VKl6QM8gQFdAjUKT0owJqUb0iZ2YqblIr+IHLIFHddhwOHB76cg
ViLKtbJ7Y43aShdQUE2t8Ibth+ywba3EDBaieqkz4Q928k//kScGSal6nxqE4mhR
AZ4ca5euEpADicacHZmKNSbtgeBYvhsC/c4ly27hdLPQjpAGOT/axPoaIfUxIe9H
PKIJx4VC14A2u4tS1A6Q
=1PBB
-----END PGP SIGNATURE-----
Marek Vasut - Dec. 13, 2012, 1:12 p.m.
Dear Richard Genoud,

> 2012/12/13 Marek Vasut <marex@denx.de>:
> > Dear Richard Genoud,
> > 
> >> ifdefs in the code are making it harder to read.
> >> The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
> >> (the code is discarded by the compiler and linker instead of the
> >> preprocessor.)
> >> 
> >> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
> >> defined.
> >> 
> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >> ---
> >> 
> >>  fs/fat/fat.c       |   55
> >> 
> >> +++++++++++++++++++++++++++------------------------ fs/fat/fat_write.c |
> >> 11 ++-------
> >> 
> >>  2 files changed, 32 insertions(+), 34 deletions(-)
> >> 
> >> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> >> index 393c378..c79e3e3 100644
> >> --- a/fs/fat/fat.c
> >> +++ b/fs/fat/fat.c
> >> @@ -34,6 +34,12 @@
> >> 
> >>  #include <malloc.h>
> >>  #include <linux/compiler.h>
> >> 
> >> +#ifdef CONFIG_SUPPORT_VFAT
> >> +#define VFAT_ENABLED 1
> >> +#else
> >> +#define VFAT_ENABLED 0
> >> +#endif
> > 
> > [...]
> > 
> > Make it static const int maybe ?
> 
> I hesitate to make them static const, I can't figure why it's better
> to use static const variable instead of defines.
> Could you enlighten me ?

Because you get the type-checking. Preprocessor is evil ;-)

Best regards,
Marek Vasut
Richard Genoud - Dec. 13, 2012, 1:15 p.m.
2012/12/13 Tom Rini <trini@ti.com>:
> Note that we don't use --gc-sections on all archs so I'm not sure we
> discard the unused VFAT functions on say ARM.
>
I tested it on at91sam9x5ek, and the vfat functions are not present in
the System.map.

The problem is in my commit message: the linker doesn"t have anything
to do with that because all discarded functions are static.

So I think it's ok on all archs (but maybe not with -O0)

Richard
Richard Genoud - Dec. 13, 2012, 1:16 p.m.
2012/12/13 Marek Vasut <marex@denx.de>:
> Dear Richard Genoud,
>
>> 2012/12/13 Marek Vasut <marex@denx.de>:
>> > Dear Richard Genoud,
>> >
>> >> ifdefs in the code are making it harder to read.
>> >> The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
>> >> (the code is discarded by the compiler and linker instead of the
>> >> preprocessor.)
>> >>
>> >> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
>> >> defined.
>> >>
>> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> >> ---
>> >>
>> >>  fs/fat/fat.c       |   55
>> >>
>> >> +++++++++++++++++++++++++++------------------------ fs/fat/fat_write.c |
>> >> 11 ++-------
>> >>
>> >>  2 files changed, 32 insertions(+), 34 deletions(-)
>> >>
>> >> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> >> index 393c378..c79e3e3 100644
>> >> --- a/fs/fat/fat.c
>> >> +++ b/fs/fat/fat.c
>> >> @@ -34,6 +34,12 @@
>> >>
>> >>  #include <malloc.h>
>> >>  #include <linux/compiler.h>
>> >>
>> >> +#ifdef CONFIG_SUPPORT_VFAT
>> >> +#define VFAT_ENABLED 1
>> >> +#else
>> >> +#define VFAT_ENABLED 0
>> >> +#endif
>> >
>> > [...]
>> >
>> > Make it static const int maybe ?
>>
>> I hesitate to make them static const, I can't figure why it's better
>> to use static const variable instead of defines.
>> Could you enlighten me ?
>
> Because you get the type-checking. Preprocessor is evil ;-)
>
> Best regards,
> Marek Vasut

Ok ! Thanks, I'll resend it with the change.


Richard.
Marek Vasut - Dec. 13, 2012, 1:19 p.m.
Dear Richard Genoud,

> 2012/12/13 Marek Vasut <marex@denx.de>:
> > Dear Richard Genoud,
> > 
> >> 2012/12/13 Marek Vasut <marex@denx.de>:
> >> > Dear Richard Genoud,
> >> > 
> >> >> ifdefs in the code are making it harder to read.
> >> >> The use of simple if(VFAT_ENABLED) makes no more code and is cleaner.
> >> >> (the code is discarded by the compiler and linker instead of the
> >> >> preprocessor.)
> >> >> 
> >> >> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
> >> >> defined.
> >> >> 
> >> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >> >> ---
> >> >> 
> >> >>  fs/fat/fat.c       |   55
> >> >> 
> >> >> +++++++++++++++++++++++++++------------------------
> >> >> fs/fat/fat_write.c | 11 ++-------
> >> >> 
> >> >>  2 files changed, 32 insertions(+), 34 deletions(-)
> >> >> 
> >> >> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> >> >> index 393c378..c79e3e3 100644
> >> >> --- a/fs/fat/fat.c
> >> >> +++ b/fs/fat/fat.c
> >> >> @@ -34,6 +34,12 @@
> >> >> 
> >> >>  #include <malloc.h>
> >> >>  #include <linux/compiler.h>
> >> >> 
> >> >> +#ifdef CONFIG_SUPPORT_VFAT
> >> >> +#define VFAT_ENABLED 1
> >> >> +#else
> >> >> +#define VFAT_ENABLED 0
> >> >> +#endif
> >> > 
> >> > [...]
> >> > 
> >> > Make it static const int maybe ?
> >> 
> >> I hesitate to make them static const, I can't figure why it's better
> >> to use static const variable instead of defines.
> >> Could you enlighten me ?
> > 
> > Because you get the type-checking. Preprocessor is evil ;-)
> > 
> > Best regards,
> > Marek Vasut
> 
> Ok ! Thanks, I'll resend it with the change.

Thanks!

Yes, preprocessor eats kittens too ;-)

Best regards,
Marek Vasut
Marek Vasut - Dec. 13, 2012, 1:51 p.m.
Dear Tom Rini,

[...]

> Note that we don't use --gc-sections on all archs so I'm not sure we
> discard the unused VFAT functions on say ARM.

Valid point, Albert?

Best regards,
Marek Vasut
Richard Genoud - Dec. 13, 2012, 2:24 p.m.
2012/12/13 Marek Vasut <marex@denx.de>:
> Dear Tom Rini,
>
> [...]
>
>> Note that we don't use --gc-sections on all archs so I'm not sure we
>> discard the unused VFAT functions on say ARM.
>
> Valid point, Albert?
>
> Best regards,
> Marek Vasut

I check and the code is discarded (on ARM at91sam9x5ek).
This is the case because the function to discard are all in the same
file (fat.c or fat_write.c).
The compiler will discard unused function with -Os. (but not with -O0)
Tom Rini - Dec. 13, 2012, 2:41 p.m.
On Thu, Dec 13, 2012 at 03:24:08PM +0100, Richard Genoud wrote:
> 2012/12/13 Marek Vasut <marex@denx.de>:
> > Dear Tom Rini,
> >
> > [...]
> >
> >> Note that we don't use --gc-sections on all archs so I'm not sure we
> >> discard the unused VFAT functions on say ARM.
> >
> > Valid point, Albert?
> >
> > Best regards,
> > Marek Vasut
> 
> I check and the code is discarded (on ARM at91sam9x5ek).
> This is the case because the function to discard are all in the same
> file (fat.c or fat_write.c).
> The compiler will discard unused function with -Os. (but not with -O0)

Thanks for checking.

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 393c378..c79e3e3 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -34,6 +34,12 @@ 
 #include <malloc.h>
 #include <linux/compiler.h>
 
+#ifdef CONFIG_SUPPORT_VFAT
+#define VFAT_ENABLED 1
+#else
+#define VFAT_ENABLED 0
+#endif
+
 /*
  * Convert a string to lowercase.
  */
@@ -441,7 +447,6 @@  getit:
 	} while (1);
 }
 
-#ifdef CONFIG_SUPPORT_VFAT
 /*
  * Extract the file name information from 'slotptr' into 'l_name',
  * starting at l_name[*idx].
@@ -576,7 +581,6 @@  static __u8 mkcksum(const char name[8], const char ext[3])
 
 	return ret;
 }
-#endif	/* CONFIG_SUPPORT_VFAT */
 
 /*
  * Get the directory entry associated with 'filename' from the directory
@@ -617,8 +621,8 @@  static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
 				continue;
 			}
 			if ((dentptr->attr & ATTR_VOLUME)) {
-#ifdef CONFIG_SUPPORT_VFAT
-				if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
+				if (VFAT_ENABLED &&
+				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
 				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
 					prevcksum = ((dir_slot *)dentptr)->alias_checksum;
 					get_vfatname(mydata, curclust,
@@ -658,9 +662,7 @@  static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
 						continue;
 					}
 					debug("vfatname: |%s|\n", l_name);
-				} else
-#endif
-				{
+				} else {
 					/* Volume label or VFAT entry */
 					dentptr++;
 					continue;
@@ -674,14 +676,15 @@  static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
 				debug("Dentname == NULL - %d\n", i);
 				return NULL;
 			}
-#ifdef CONFIG_SUPPORT_VFAT
-			__u8 csum = mkcksum(dentptr->name, dentptr->ext);
-			if (dols && csum == prevcksum) {
-				prevcksum = 0xffff;
-				dentptr++;
-				continue;
+			if (VFAT_ENABLED) {
+				__u8 csum = mkcksum(dentptr->name, dentptr->ext);
+				if (dols && csum == prevcksum) {
+					prevcksum = 0xffff;
+					dentptr++;
+					continue;
+				}
 			}
-#endif
+
 			get_name(dentptr, s_name);
 			if (dols) {
 				int isdir = (dentptr->attr & ATTR_DIR);
@@ -884,9 +887,9 @@  do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 		return -1;
 	}
 
-#ifdef CONFIG_SUPPORT_VFAT
-	debug("VFAT Support enabled\n");
-#endif
+	if (VFAT_ENABLED)
+		debug("VFAT Support enabled\n");
+
 	debug("FAT%d, fat_sect: %d, fatlength: %d\n",
 	       mydata->fatsize, mydata->fat_sect, mydata->fatlength);
 	debug("Rootdir begins at cluster: %d, sector: %d, offset: %x\n"
@@ -952,10 +955,12 @@  do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 				continue;
 			}
 
-			csum = mkcksum(dentptr->name, dentptr->ext);
+			if (VFAT_ENABLED)
+				csum = mkcksum(dentptr->name, dentptr->ext);
+
 			if (dentptr->attr & ATTR_VOLUME) {
-#ifdef CONFIG_SUPPORT_VFAT
-				if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
+				if (VFAT_ENABLED &&
+				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
 				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
 					prevcksum =
 						((dir_slot *)dentptr)->alias_checksum;
@@ -999,9 +1004,7 @@  do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 					}
 					debug("Rootvfatname: |%s|\n",
 					       l_name);
-				} else
-#endif
-				{
+				} else {
 					/* Volume label or VFAT entry */
 					dentptr++;
 					continue;
@@ -1015,13 +1018,13 @@  do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 				}
 				goto exit;
 			}
-#ifdef CONFIG_SUPPORT_VFAT
-			else if (dols == LS_ROOT && csum == prevcksum) {
+			else if (VFAT_ENABLED &&
+				 dols == LS_ROOT && csum == prevcksum) {
 				prevcksum = 0xffff;
 				dentptr++;
 				continue;
 			}
-#endif
+
 			get_name(dentptr, s_name);
 
 			if (dols == LS_ROOT) {
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 4a1bda0..ed3eaa0 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -248,7 +248,6 @@  static __u32 get_fatent_value(fsdata *mydata, __u32 entry)
 	return ret;
 }
 
-#ifdef CONFIG_SUPPORT_VFAT
 /*
  * Set the file name information from 'name' into 'slotptr',
  */
@@ -468,8 +467,6 @@  get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
 	return 0;
 }
 
-#endif
-
 /*
  * Set the entry at index 'entry' in a FAT (16/32) table.
  */
@@ -853,16 +850,14 @@  static dir_entry *find_directory_entry(fsdata *mydata, int startsect,
 				continue;
 			}
 			if ((dentptr->attr & ATTR_VOLUME)) {
-#ifdef CONFIG_SUPPORT_VFAT
-				if ((dentptr->attr & ATTR_VFAT) &&
+				if (VFAT_ENABLED &&
+				    (dentptr->attr & ATTR_VFAT) &&
 				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
 					get_long_file_name(mydata, curclust,
 						     get_dentfromdir_block,
 						     &dentptr, l_name);
 					debug("vfatname: |%s|\n", l_name);
-				} else
-#endif
-				{
+				} else {
 					/* Volume label or VFAT entry */
 					dentptr++;
 					if (is_next_clust(mydata, dentptr))