[syslinux:pathbased] core/fs: clean up, fix and improve the internal readdir() interfaces

syslinux-bot for H. Peter Anvin hpa at zytor.com
Sat Mar 6 14:54:14 PST 2010


Commit-ID:  01ac07292bb174312d82fbe8c2849f315eca60ba
Gitweb:     http://syslinux.zytor.com/commit/01ac07292bb174312d82fbe8c2849f315eca60ba
Author:     H. Peter Anvin <hpa at zytor.com>
AuthorDate: Sat, 6 Mar 2010 14:51:08 -0800
Committer:  H. Peter Anvin <hpa at zytor.com>
CommitDate: Sat, 6 Mar 2010 14:51:08 -0800

core/fs: clean up, fix and improve the internal readdir() interfaces

- Avoid a completely unnecessary malloc/free pair by passing a pointer
  to the filesystem driver;
- Make sure d_reclen is always set correctly;
- Make sure the d_type field is set correctly.

Signed-off-by: H. Peter Anvin <hpa at zytor.com>


---
 core/fs/ext2/ext2.c       |   13 +++--------
 core/fs/fat/fat.c         |   50 ++++++++++++++++++--------------------------
 core/fs/iso9660/iso9660.c |   16 ++++---------
 core/fs/readdir.c         |   14 +++---------
 core/include/fs.h         |   15 +++++++++++-
 5 files changed, 47 insertions(+), 61 deletions(-)

diff --git a/core/fs/ext2/ext2.c b/core/fs/ext2/ext2.c
index 14457af..44f28f9 100644
--- a/core/fs/ext2/ext2.c
+++ b/core/fs/ext2/ext2.c
@@ -233,36 +233,31 @@ static int ext2_readlink(struct inode *inode, char *buf)
 /*
  * Read one directory entry at a time
  */
-static struct dirent *ext2_readdir(struct file *file)
+static int ext2_readdir(struct file *file, struct dirent *dirent)
 {
     struct fs_info *fs = file->fs;
     struct inode *inode = file->inode;
-    struct dirent *dirent;
     const struct ext2_dir_entry *de;
     const char *data;
     block_t index = file->offset >> fs->block_shift;
 
     if (file->offset >= inode->size)
-	return NULL;		/* End of file */
+	return -1;		/* End of file */
 
     data = ext2_get_cache(inode, index);
     de = (const struct ext2_dir_entry *)
 	(data + (file->offset & (BLOCK_SIZE(fs) - 1)));
 
-    if (!(dirent = malloc(sizeof(*dirent)))) {
-	malloc_error("dirent structure in ext2_readdir");
-	return NULL;
-    }
     dirent->d_ino = de->d_inode;
     dirent->d_off = file->offset;
-    dirent->d_reclen = de->d_rec_len;
+    dirent->d_reclen = offsetof(struct dirent, d_name) + de->d_name_len + 1;
     dirent->d_type = de->d_file_type;
     memcpy(dirent->d_name, de->d_name, de->d_name_len);
     dirent->d_name[de->d_name_len] = '\0';
 
     file->offset += de->d_rec_len;  /* Update for next reading */
 
-    return dirent;
+    return 0;
 }
 
 /*
diff --git a/core/fs/fat/fat.c b/core/fs/fat/fat.c
index e21a623..ee48f73 100644
--- a/core/fs/fat/fat.c
+++ b/core/fs/fat/fat.c
@@ -574,10 +574,9 @@ static struct inode *vfat_iget(const char *dname, struct inode *parent)
     return vfat_find_entry(dname, parent);
 }
 
-static struct dirent * vfat_readdir(struct file *file)
+static int vfat_readdir(struct file *file, struct dirent *dirent)
 {
     struct fs_info *fs = file->fs;
-    struct dirent *dirent;
     const struct fat_dir_entry *de;
     const char *data;
     const struct fat_long_name_entry *long_de;
@@ -586,12 +585,13 @@ static struct dirent * vfat_readdir(struct file *file)
 
     uint16_t long_name[261];	/* == 20*13 + 1 (to guarantee null) */
     char filename[261];
+    int name_len = 0;
 
     uint8_t vfat_init, vfat_next, vfat_csum;
     uint8_t id;
     int entries_left;
     int checksum;
-    int long_entry = 0;
+    bool long_entry = false;
     int sec_off = file->offset & ((1 << fs->sector_shift) - 1);
 
     data = get_cache(fs->fs_dev, sector);
@@ -601,9 +601,9 @@ static struct dirent * vfat_readdir(struct file *file)
     vfat_next = vfat_csum = 0xff;
 
     while (1) {
-	while(entries_left--) {
+	while (entries_left--) {
 	    if (de->name[0] == 0)
-		return NULL;
+		return -1;	/* End of directory */
 	    if ((uint8_t)de->name[0] == 0xe5)
 		goto invalid;
 
@@ -626,8 +626,7 @@ static struct dirent * vfat_readdir(struct file *file)
 		    /* ZERO the long_name buffer */
 		    memset(long_name, 0, sizeof long_name);
 		} else {
-		    if (long_de->checksum != vfat_csum ||
-			id != vfat_next)
+		    if (long_de->checksum != vfat_csum || id != vfat_next)
 			goto invalid;
 		}
 
@@ -637,15 +636,12 @@ static struct dirent * vfat_readdir(struct file *file)
 		copy_long_chunk(long_name + id*13, de);
 
 		if (id == 0) {
-		    int longlen =
-			vfat_cvt_longname(filename, long_name);
-		    if (longlen > 0 && longlen < sizeof(dirent->d_name))
-			long_entry = 1;
+		    name_len = vfat_cvt_longname(filename, long_name);
+		    if (name_len > 0 && name_len < sizeof(dirent->d_name))
+			long_entry = true;
 		}
 
-		de++;
-		file->offset += sizeof(struct fat_dir_entry);
-		continue;     /* Try the next entry */
+		goto next;
 	    } else {
 		/*
 		 * It's a short entry
@@ -653,11 +649,8 @@ static struct dirent * vfat_readdir(struct file *file)
 		if (de->attr & 0x08) /* ignore volume labels */
 		    goto invalid;
 
-		if (long_entry == 1) {
-		    /* Got a long entry */
-		    checksum = get_checksum(de->name);
-		    if (checksum == vfat_csum)
-			goto got;
+		if (long_entry && get_checksum(de->name) == vfat_csum) {
+		   /* Got a long entry */
 		} else {
 		    /* Use the shortname */
 		    int i;
@@ -684,12 +677,14 @@ static struct dirent * vfat_readdir(struct file *file)
 			}
 		    }
 		    *p = '\0';
-
-		    goto got;
+		    name_len = p - filename;
 		}
+		goto got;	/* Got something one way or the other */
 	    }
 
 	invalid:
+	    long_entry = false;
+	next:
 	    de++;
 	    file->offset += sizeof(struct fat_dir_entry);
 	}
@@ -697,25 +692,22 @@ static struct dirent * vfat_readdir(struct file *file)
 	/* Try with the next sector */
 	sector = next_sector(file);
 	if (!sector)
-	    return NULL;
+	    return -1;
 	de = get_cache(fs->fs_dev, sector);
 	entries_left = 1 << (fs->sector_shift - 5);
     }
 
 got:
-    if (!(dirent = malloc(sizeof(*dirent)))) {
-	malloc_error("dirent structure in vfat_readdir");
-	return NULL;
-    }
+    name_len++;			/* Include final null */
     dirent->d_ino = de->first_cluster_low | (de->first_cluster_high << 16);
     dirent->d_off = file->offset;
-    dirent->d_reclen = 0;
+    dirent->d_reclen = offsetof(struct dirent, d_name) + name_len;
     dirent->d_type = get_inode_mode(de->attr);
-    strcpy(dirent->d_name, filename);
+    memcpy(dirent->d_name, filename, name_len);
 
     file->offset += sizeof(*de);  /* Update for next reading */
 
-    return dirent;
+    return 0;
 }
 
 /* Load the config file, return 1 if failed, or 0 */
diff --git a/core/fs/iso9660/iso9660.c b/core/fs/iso9660/iso9660.c
index 2362293..bd5f406 100644
--- a/core/fs/iso9660/iso9660.c
+++ b/core/fs/iso9660/iso9660.c
@@ -235,12 +235,11 @@ static struct inode *iso_iget(const char *dname, struct inode *parent)
     return iso_get_inode(parent->fs, de);
 }
 
-static struct dirent *iso_readdir(struct file *file)
+static int iso_readdir(struct file *file, struct dirent *dirent)
 {
     struct fs_info *fs = file->fs;
     struct inode *inode = file->inode;
     const struct iso_dir_entry *de;
-    struct dirent *dirent;
     const char *data = NULL;
     
     while (1) {
@@ -249,7 +248,7 @@ static struct dirent *iso_readdir(struct file *file)
 	if (!data) {
 	    uint32_t i = file->offset >> BLOCK_SHIFT(fs);
 	    if (i >= inode->blocks)
-		return NULL;
+		return -1;
 	    data = get_cache(fs->fs_dev, PVT(inode)->lba + i);
 	}
 	de = (const struct iso_dir_entry *)(data + offset);
@@ -263,20 +262,15 @@ static struct dirent *iso_readdir(struct file *file)
 	break;
     }
     
-    if (!(dirent = malloc(sizeof(*dirent)))) {
-	malloc_error("dirent structure in iso_readdir");
-	return NULL;
-    }
-    
     dirent->d_ino = 0;           /* Inode number is invalid to ISO fs */
     dirent->d_off = file->offset;
     dirent->d_type = get_inode_mode(de->flags);
-    dirent->d_reclen = iso_convert_name(dirent->d_name,
-					de->name, de->name_len);
+    dirent->d_reclen = offsetof(struct dirent, d_name) + 1 +
+	iso_convert_name(dirent->d_name, de->name, de->name_len);
 
     file->offset += de->length;  /* Update for next reading */
     
-    return dirent;
+    return 0;
 }
 
 /* Load the config file, return 1 if failed, or 0 */
diff --git a/core/fs/readdir.c b/core/fs/readdir.c
index 7fa9934..d2b112b 100644
--- a/core/fs/readdir.c
+++ b/core/fs/readdir.c
@@ -25,19 +25,13 @@ DIR *opendir(const char *path)
 struct dirent *readdir(DIR *dir)
 {
     static struct dirent buf;
-    struct dirent *de = NULL;
     struct file *dd_dir = (struct file *)dir;
+    int rv = -1;
     
     if (dd_dir)
-	de = dd_dir->fs->fs_ops->readdir(dd_dir);
-    
-    if (de) {
-	memcpy(&buf, de, sizeof *de);
-	free(de);
-	return &buf;
-    } else {
-	return NULL;
-    }
+	rv = dd_dir->fs->fs_ops->readdir(dd_dir, &buf);
+
+    return rv < 0 ? NULL : &buf;
 }
 
 /*
diff --git a/core/include/fs.h b/core/include/fs.h
index 332607b..187a55d 100644
--- a/core/include/fs.h
+++ b/core/include/fs.h
@@ -68,12 +68,23 @@ struct fs_ops {
     int	     (*readlink)(struct inode *, char *);
 
     /* the _dir_ stuff */
-    struct dirent * (*readdir)(struct file *);
+    int	     (*readdir)(struct file *, struct dirent *);
 
     int      (*next_extent)(struct inode *, uint32_t);
 };
 
-enum inode_mode {I_FILE, I_DIR, I_SYMLINK};
+/* XXX: merge this with enum dirent_types */
+enum inode_mode {
+    I_UNKNOWN	=  0,
+    I_FIFO	=  1,
+    I_CHR	=  2,
+    I_DIR	=  4,
+    I_BLK	=  6,
+    I_FILE	=  8,
+    I_SYMLINK	= 10,
+    I_SOCK	= 12,
+    I_WHT	= 14,
+};
 
 /*
  * Extent structure: contains the mapping of some chunk of a file



More information about the Syslinux-commits mailing list