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

login
register
mail settings
Submitter Richard Genoud
Date Dec. 13, 2012, 1:30 p.m.
Message ID <1355405410-2783-1-git-send-email-richard.genoud@gmail.com>
Download mbox | patch
Permalink /patch/205838/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Richard Genoud - Dec. 13, 2012, 1:30 p.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 instead of the preprocessor.)
NB: if -O0 is used, the code won't be discarded

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(-)
Tom Rini - Feb. 4, 2013, 4:42 p.m.
On Thu, Dec 13, 2012 at 03:30:10AM -0000, 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 instead of the preprocessor.)
> NB: if -O0 is used, the code won't be discarded
> 
> and bonus, now the code compiles even if CONFIG_SUPPORT_VFAT is not
> defined.
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Applied to u-boot/master, thanks!

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 393c378..7c23b67 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
+static const int vfat_enabled = 1;
+#else
+static const int 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..e7cda17 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))