ext4: cleanup unlink_filename function

Use the same variable names as in search_dir, to make purpose of variables more obvious.

Signed-off-by Stefan Br?ns <[email protected]>

Use the same variable names as in search_dir, to make purpose of variables more obvious.

Signed-off-by Stefan Br?ns <[email protected]>

Reviewed-by Tom Rini <[email protected]>

The direntlen checks were quite bogus, i.e. the loop termination used "len + offset == blocksize" (exact match only), and checked for a direntlen less than 0. The latter can never happen as the len is unsigned, this has been reported by Coverity, CID 153384.

Use the same code as in search_dir for directory traversal. This code has the correct checks for direntlen >= sizeof(struct dirent), and offset < blocksize.

Signed-off-by Stefan Br?ns <[email protected]>

The direntlen checks were quite bogus, i.e. the loop termination used "len + offset == blocksize" (exact match only), and checked for a direntlen less than 0. The latter can never happen as the len is unsigned, this has been reported by Coverity, CID 153384.

Use the same code as in search_dir for directory traversal. This code has the correct checks for direntlen >= sizeof(struct dirent), and offset < blocksize.

Signed-off-by Stefan Br?ns <[email protected]>

---
 fs/ext4/ext4_common.c | 45

+++++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 28 deletions(-)

fs/ext4/ext4_common.c +8 -14

index 699a640..11be0b7 100644
@@ -854,16 +854,15 @@ fail:
 
 static int unlink_filename(char *filename, unsigned int blknr)
 {
-	int templength = 0;
-	int status, inodeno;
-	int found = 0;
+	int status;
+	int inodeno = 0;
 	int offset;
 	char *block_buffer = NULL;
 	struct ext2_dirent *dir = NULL;
-	struct ext2_dirent *previous_dir = NULL;
-	char *ptr = NULL;
+	struct ext2_dirent *previous_dir;
 	struct ext_filesystem *fs = get_fs();
 	int ret = -1;
+	char *direntname;
 
 	block_buffer = zalloc(fs->blksz);
 	if (!block_buffer)

@@ -877,20 +876,18 @@ static int unlink_filename(char *filename,
 unsigned int blknr) 
 	if (ext4fs_log_journal(block_buffer, blknr))
 		goto fail;
-	dir = (struct ext2_dirent *)block_buffer;
-	ptr = (char *)dir;
 	offset = 0;
-	while (le16_to_cpu(dir->direntlen) >= 0) {
-		/*
-		 * blocksize-offset because last
-		 * directory length i.e., *dir->direntlen
-		 * is free availble space in the block that
-		 * means it is a last entry of directory entry
-		 */
+	do {
+		previous_dir = dir;
+		dir = (struct ext2_dirent *)(block_buffer + offset);
+		direntname = (char *)(dir) + sizeof(struct

ext2_dirent); +

fs/ext4/ext4_common.c +4 -0

+		int direntlen = le16_to_cpu(dir->direntlen);
+		if (direntlen < sizeof(struct ext2_dirent))
+			break;
+
 		if (dir->inode && (strlen(filename) == dir->namelen)

&&

fs/ext4/ext4_common.c +1 -3

-		    (strncmp(ptr + sizeof(struct ext2_dirent),
-			     filename, dir->namelen) == 0)) {
-			printf("file found, deleting\n");
+		    (strncmp(direntname, filename, dir->namelen) ==

0)) { inodeno = le32_to_cpu(dir->inode);

fs/ext4/ext4_common.c +3 -11

 			if (previous_dir) {
 				uint16_t new_len;

@@ -900,23 +897,15 @@ static int unlink_filename(char *filename,
 unsigned int blknr) } else {
 				dir->inode = 0;
 			}
-			found = 1;
 			break;
 		}
 
-		if (fs->blksz - offset ==
 le16_to_cpu(dir->direntlen))
-			break;
+		offset += direntlen;
 
-		/* traversing the each directory entry */
-		templength = le16_to_cpu(dir->direntlen);
-		offset = offset + templength;
-		previous_dir = dir;
-		dir = (struct ext2_dirent *)((char *)dir +
 templength);
-		ptr = (char *)dir;
-	}
+	} while (offset < fs->blksz);
 
+	if (inodeno > 0) {
 
-	if (found == 1) {
 		if (ext4fs_put_metadata(block_buffer, blknr))

The direntlen checks were quite bogus, i.e. the loop termination used "len + offset == blocksize" (exact match only), and checked for a direntlen less than 0. The latter can never happen as the len is unsigned, this has been reported by Coverity, CID 153384.

Use the same code as in search_dir for directory traversal. This code has the correct checks for direntlen >= sizeof(struct dirent), and offset < blocksize.

Signed-off-by Stefan Br?ns <[email protected]>

Reported-by Coverity (CID: 153383, 153384)

Reviewed-by Tom Rini <[email protected]>

Reviewed-by Lukasz Majewski <[email protected]>

Instead of creating a journal entry for each directory block, even if the block is unmodified, only log the modified block.

Signed-off-by Stefan Br?ns <[email protected]>

Instead of creating a journal entry for each directory block, even if the block is unmodified, only log the modified block.

Signed-off-by Stefan Br?ns <[email protected]>

---
 fs/ext4/ext4_common.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

fs/ext4/ext4_common.c +0 -2

index 11be0b7..ec02eec 100644
@@ -874,8 +874,6 @@ static int unlink_filename(char *filename,
 unsigned int blknr) if (status == 0)
 		goto fail;
 
-	if (ext4fs_log_journal(block_buffer, blknr))
-		goto fail;
 	offset = 0;
 	do {
 		previous_dir = dir;

@@ -889,14 +887,6 @@ static int unlink_filename(char *filename,

unsigned int blknr) if (dir->inode && (strlen(filename) == dir->namelen) && (strncmp(direntname, filename, dir->namelen) == 0)) {

fs/ext4/ext4_common.c +0 -3

 			inodeno = le32_to_cpu(dir->inode);
-			if (previous_dir) {
-				uint16_t new_len;
-				new_len =

le16_to_cpu(previous_dir->direntlen);

fs/ext4/ext4_common.c +0 -1

-				new_len +=

le16_to_cpu(dir->direntlen);

fs/ext4/ext4_common.c +0 -1

-				previous_dir->direntlen =

cpu_to_le16(new_len);

fs/ext4/ext4_common.c +13 -3

-			} else {
-				dir->inode = 0;
-			}
 			break;
 		}

@@ -905,7 +895,20 @@ static int unlink_filename(char *filename,
 unsigned int blknr) } while (offset < fs->blksz);
 
 	if (inodeno > 0) {
+		printf("file found, deleting\n");
+		if (ext4fs_log_journal(block_buffer, blknr))
+			goto fail;
 
+		if (previous_dir) {
+			/* merge dir entry with predecessor */
+			uint16_t new_len;
+			new_len =
 le16_to_cpu(previous_dir->direntlen);
+			new_len += le16_to_cpu(dir->direntlen);
+			previous_dir->direntlen =
 cpu_to_le16(new_len);
+		} else {
+			/* invalidate dir entry */
+			dir->inode = 0;
+		}
 		if (ext4fs_put_metadata(block_buffer, blknr))

Instead of creating a journal entry for each directory block, even if the block is unmodified, only log the modified block.

Signed-off-by Stefan Br?ns <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Reviewed-by Lukasz Majewski <[email protected]>