From 868d26ab9b54545a67150ba59a5aa9d47cb4e2d8 Mon Sep 17 00:00:00 2001

From: Lukas Czerner <lczerner@redhat.com>

Date: Thu, 20 Feb 2014 16:02:29 +0100

Subject: [PATCH 01/16] e2fsprogs: introduce ext2fs_close_free() helper

commit 47fee2ef6a23ae06f680336ffde57caa64604a4c

Currently there are many uses of ext2fs_close() which might be wrong.

First of all ext2fs_close() does not set the ext2_filsys pointer to NULL

so the caller is responsible for clearing it, however there are some

cases there we do not do it.

Second of all very small number of users of ext2fs_close() actually

check the return value. If there is a problem in ext2fs_close() it will

not even free the ext2_filsys structure, but majority of users expect it

to do so.

To fix both problems this commit introduces a new helper

ext2fs_close_free() which will not only check for the return value and

free the ext2_filsys structure if the call to ext2fs_close2() failed,

but it will also set the ext2_filsys pointer to NULL.

Replace every use of ext2fs_close() in e2fsprogs tools with

ext2fs_close_free() - there is no real reason to keep using

ext2fs_close().

Signed-off-by: Lukas Czerner <lczerner@redhat.com>

---

 debugfs/debugfs.c        |  6 ++----

 e2fsck/scantest.c        |  2 +-

 e2fsck/unix.c            | 20 ++++++++------------

 e2fsck/util.c            |  2 +-

 lib/ext2fs/closefs.c     | 12 ++++++++++++

 lib/ext2fs/ext2fs.h      |  1 +

 lib/ext2fs/mkjournal.c   |  2 +-

 lib/ext2fs/tst_bitmaps.c | 12 ++++--------

 misc/dumpe2fs.c          |  6 +++---

 misc/e2freefrag.c        |  2 +-

 misc/e2image.c           |  4 ++--

 misc/e4defrag.c          |  2 +-

 misc/mke2fs.c            |  8 ++++----

 misc/tune2fs.c           |  6 +++---

 resize/main.c            |  2 +-

 resize/resize2fs.c       |  2 +-

 16 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c

index cf7670bc..2082309b 100644

--- a/debugfs/debugfs.c

+++ b/debugfs/debugfs.c

@@ -131,10 +131,9 @@ static void open_filesystem(char *device, int open_flags, blk64_t superblock,

 	return;

 errout:

-	retval = ext2fs_close(current_fs);

+	retval = ext2fs_close_free(&current_fs);

 	if (retval)

 		com_err(device, retval, "while trying to close filesystem");

-	current_fs = NULL;

 }

 void do_open_filesys(int argc, char **argv)

@@ -237,10 +236,9 @@ static void close_filesystem(NOARGS)

 		if (retval)

 			com_err("ext2fs_write_block_bitmap", retval, 0);

 	}

-	retval = ext2fs_close(current_fs);

+	retval = ext2fs_close_free(&current_fs);

 	if (retval)

 		com_err("ext2fs_close", retval, 0);

-	current_fs = NULL;

 	return;

 }

diff --git a/e2fsck/scantest.c b/e2fsck/scantest.c

index 16380b31..61311410 100644

--- a/e2fsck/scantest.c

+++ b/e2fsck/scantest.c

@@ -133,7 +133,7 @@ int main (int argc, char *argv[])

 	}

-	ext2fs_close(fs);

+	ext2fs_close_free(&fs);

 	print_resource_track(&global_rtrack);

diff --git a/e2fsck/unix.c b/e2fsck/unix.c

index d94d5dcd..5fcc9d99 100644

--- a/e2fsck/unix.c

+++ b/e2fsck/unix.c

@@ -456,8 +456,7 @@ static void check_if_skip(e2fsck_t ctx)

 	}

 	log_out(ctx, "\n");

 skip:

-	ext2fs_close(fs);

-	ctx->fs = NULL;

+	ext2fs_close_free(&fs);

 	e2fsck_free_context(ctx);

 	exit(FSCK_OK);

 }

@@ -1303,12 +1302,12 @@ restart:

 			orig_superblock = ctx->superblock;

 			get_backup_sb(ctx, fs, ctx->filesystem_name, io_ptr);

 			if (fs)

-				ext2fs_close(fs);

+				ext2fs_close_free(&fs);

 			orig_retval = retval;

 			retval = try_open_fs(ctx, flags, io_ptr, &fs);

 			if ((orig_retval == 0) && retval != 0) {

 				if (fs)

-					ext2fs_close(fs);

+					ext2fs_close_free(&fs);

 				log_out(ctx, _("%s: %s while using the "

 					       "backup blocks"),

 					ctx->program_name,

@@ -1402,7 +1401,7 @@ failure:

 		 * reopen the filesystem after we get the device size.

 		 */

 		if (pctx.errcode == EBUSY) {

-			ext2fs_close(fs);

+			ext2fs_close_free(&fs);

 			need_restart++;

 			pctx.errcode =

 				ext2fs_get_device_size2(ctx->filesystem_name,

@@ -1459,8 +1458,7 @@ failure:

 		/*

 		 * Restart in order to reopen fs but this time start mmp.

 		 */

-		ext2fs_close(fs);

-		ctx->fs = NULL;

+		ext2fs_close_free(&fs);

 		flags &= ~EXT2_FLAG_SKIP_MMP;

 		goto restart;

 	}

@@ -1510,8 +1508,7 @@ failure:

 					ctx->device_name);

 				fatal_error(ctx, 0);

 			}

-			ext2fs_close(ctx->fs);

-			ctx->fs = 0;

+			ext2fs_close_free(&ctx->fs);

 			ctx->flags |= E2F_FLAG_RESTARTED;

 			goto restart;

 		}

@@ -1690,7 +1687,7 @@ no_journal:

 				_("while resetting context"));

 			fatal_error(ctx, 0);

 		}

-		ext2fs_close(fs);

+		ext2fs_close_free(&fs);

 		goto restart;

 	}

 	if (run_result & E2F_FLAG_CANCEL) {

@@ -1772,8 +1769,7 @@ no_journal:

 	io_channel_flush(ctx->fs->io);

 	print_resource_track(ctx, NULL, &ctx->global_rtrack, ctx->fs->io);

-	ext2fs_close(fs);

-	ctx->fs = NULL;

+	ext2fs_close_free(&fs);

 	free(ctx->journal_name);

 	e2fsck_free_context(ctx);

diff --git a/e2fsck/util.c b/e2fsck/util.c

index 9f920b2c..14c9ad48 100644

--- a/e2fsck/util.c

+++ b/e2fsck/util.c

@@ -319,7 +319,7 @@ void preenhalt(e2fsck_t ctx)

 	if (fs != NULL) {

 		fs->super->s_state |= EXT2_ERROR_FS;

 		ext2fs_mark_super_dirty(fs);

-		ext2fs_close(fs);

+		ext2fs_close_free(&fs);

 	}

 	exit(FSCK_UNCORRECTED);

 }

diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c

index 000ebd87..4db9e194 100644

--- a/lib/ext2fs/closefs.c

+++ b/lib/ext2fs/closefs.c

@@ -437,6 +437,18 @@ errout:

 	return retval;

 }

+errcode_t ext2fs_close_free(ext2_filsys *fs_ptr)

+{

+	errcode_t ret;

+	ext2_filsys fs = *fs_ptr;

+

+	ret = ext2fs_close2(fs, 0);

+	if (ret)

+		ext2fs_free(fs);

+	*fs_ptr = NULL;

+	return ret;

+}

+

 errcode_t ext2fs_close(ext2_filsys fs)

 {

 	return ext2fs_close2(fs, 0);

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h

index 380608b2..643b66c0 100644

--- a/lib/ext2fs/ext2fs.h

+++ b/lib/ext2fs/ext2fs.h

@@ -925,6 +925,7 @@ extern errcode_t ext2fs_check_desc(ext2_filsys fs);

 /* closefs.c */

 extern errcode_t ext2fs_close(ext2_filsys fs);

 extern errcode_t ext2fs_close2(ext2_filsys fs, int flags);

+extern errcode_t ext2fs_close_free(ext2_filsys *fs);

 extern errcode_t ext2fs_flush(ext2_filsys fs);

 extern errcode_t ext2fs_flush2(ext2_filsys fs, int flags);

 extern int ext2fs_bg_has_super(ext2_filsys fs, dgrp_t group_block);

diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c

index d09c4589..52dc99be 100644

--- a/lib/ext2fs/mkjournal.c

+++ b/lib/ext2fs/mkjournal.c

@@ -630,7 +630,7 @@ main(int argc, char **argv)

 	if (retval) {

 		printf("Warning, had trouble writing out superblocks.\n");

 	}

-	ext2fs_close(fs);

+	ext2fs_close_free(&fs);

 	exit(0);

 }

diff --git a/lib/ext2fs/tst_bitmaps.c b/lib/ext2fs/tst_bitmaps.c

index 57bfd6c8..3a6d1bdc 100644

--- a/lib/ext2fs/tst_bitmaps.c

+++ b/lib/ext2fs/tst_bitmaps.c

@@ -187,8 +187,7 @@ static void setup_filesystem(const char *name,

 	return;

 errout:

-	ext2fs_close(test_fs);

-	test_fs = 0;

+	ext2fs_close_free(&test_fs);

 }

 void setup_cmd(int argc, char **argv)

@@ -199,10 +198,8 @@ void setup_cmd(int argc, char **argv)

 	unsigned int	type = EXT2FS_BMAP64_BITARRAY;

 	int		flags = EXT2_FLAG_64BITS;

-	if (test_fs) {

-		ext2fs_close(test_fs);

-		test_fs = 0;

-	}

+	if (test_fs)

+		ext2fs_close_free(&test_fs);

 	reset_getopt();

 	while ((c = getopt(argc, argv, "b:i:lt:")) != EOF) {

@@ -242,8 +239,7 @@ void close_cmd(int argc, char **argv)

 	if (check_fs_open(argv[0]))

 		return;

-	ext2fs_close(test_fs);

-	test_fs = 0;

+	ext2fs_close_free(&test_fs);

 }

diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c

index d4bde8e5..cc18ad83 100644

--- a/misc/dumpe2fs.c

+++ b/misc/dumpe2fs.c

@@ -614,7 +614,7 @@ int main (int argc, char ** argv)

 		if (fs->super->s_feature_incompat &

 		      EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {

 			print_journal_information(fs);

-			ext2fs_close(fs);

+			ext2fs_close_free(&fs);

 			exit(0);

 		}

 		if ((fs->super->s_feature_compat &

@@ -623,7 +623,7 @@ int main (int argc, char ** argv)

 			print_inline_journal_information(fs);

 		list_bad_blocks(fs, 0);

 		if (header_only) {

-			ext2fs_close (fs);

+			ext2fs_close_free(&fs);

 			exit (0);

 		}

 		retval = ext2fs_read_bitmaps (fs);

@@ -634,7 +634,7 @@ int main (int argc, char ** argv)

 			       error_message(retval));

 		}

 	}

-	ext2fs_close (fs);

+	ext2fs_close_free(&fs);

 	remove_error_table(&et_ext2_error_table);

 	exit (0);

 }

diff --git a/misc/e2freefrag.c b/misc/e2freefrag.c

index 612ca445..bb72c70d 100644

--- a/misc/e2freefrag.c

+++ b/misc/e2freefrag.c

@@ -215,7 +215,7 @@ static errcode_t get_chunk_info(ext2_filsys fs, struct chunk_info *info,

 static void close_device(char *device_name, ext2_filsys fs)

 {

-	int retval = ext2fs_close(fs);

+	int retval = ext2fs_close_free(&fs);

 	if (retval)

 		com_err(device_name, retval, "while closing the filesystem.\n");

diff --git a/misc/e2image.c b/misc/e2image.c

index 0537b0d8..98dafa3d 100644

--- a/misc/e2image.c

+++ b/misc/e2image.c

@@ -1415,7 +1415,7 @@ static void install_image(char *device, char *image_fn, int type)

 	}

 	close(fd);

-	ext2fs_close (fs);

+	ext2fs_close_free(&fs);

 }

 static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)

@@ -1648,7 +1648,7 @@ skip_device:

 	else

 		write_image_file(fs, fd);

-	ext2fs_close (fs);

+	ext2fs_close_free(&fs);

 	if (check)

 		printf(_("%d blocks already contained the data to be copied.\n"),

 		       skipped_blocks);

diff --git a/misc/e4defrag.c b/misc/e4defrag.c

index 1ba3c53a..2c2034cf 100644

--- a/misc/e4defrag.c

+++ b/misc/e4defrag.c

@@ -1863,7 +1863,7 @@ int main(int argc, char *argv[])

 			feature_incompat = fs->super->s_feature_incompat;

 			log_groups_per_flex = fs->super->s_log_groups_per_flex;

-			ext2fs_close(fs);

+			ext2fs_close_free(&fs);

 		}

 		switch (arg_type) {

diff --git a/misc/mke2fs.c b/misc/mke2fs.c

index 8952a5fe..2787a127 100644

--- a/misc/mke2fs.c

+++ b/misc/mke2fs.c

@@ -1662,7 +1662,7 @@ profile_error:

 		printf(_("Using journal device's blocksize: %d\n"), blocksize);

 		fs_param.s_log_block_size =

 			int_log2(blocksize >> EXT2_MIN_BLOCK_LOG_SIZE);

-		ext2fs_close(jfs);

+		ext2fs_close_free(&jfs;;

 	}

 	if (optind < argc) {

@@ -2585,7 +2585,7 @@ int main (int argc, char *argv[])

 	if (fs->super->s_feature_incompat &

 	    EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {

 		create_journal_dev(fs);

-		exit(ext2fs_close(fs) ? 1 : 0);

+		exit(ext2fs_close_free(&fs) ? 1 : 0);

 	}

 	if (bad_blocks_filename)

@@ -2702,7 +2702,7 @@ int main (int argc, char *argv[])

 		}

 		if (!quiet)

 			printf("%s", _("done\n"));

-		ext2fs_close(jfs);

+		ext2fs_close_free(&jfs;;

 		free(journal_device);

 	} else if ((journal_size) ||

 		   (fs_param.s_feature_compat &

@@ -2761,7 +2761,7 @@ no_journal:

 		       "filesystem accounting information: "));

 	checkinterval = fs->super->s_checkinterval;

 	max_mnt_count = fs->super->s_max_mnt_count;

-	retval = ext2fs_close(fs);

+	retval = ext2fs_close_free(&fs);

 	if (retval) {

 		fprintf(stderr, "%s",

 			_("\nWarning, had trouble writing out superblocks.\n"));

diff --git a/misc/tune2fs.c b/misc/tune2fs.c

index d2aa125d..1bedca20 100644

--- a/misc/tune2fs.c

+++ b/misc/tune2fs.c

@@ -682,7 +682,7 @@ static int add_journal(ext2_filsys fs)

 		fflush(stdout);

 		retval = ext2fs_add_journal_device(fs, jfs);

-		ext2fs_close(jfs);

+		ext2fs_close_free(&jfs;;

 		if (retval) {

 			com_err(program_name, retval,

 				_("while adding filesystem to journal on %s"),

@@ -1987,7 +1987,7 @@ retry_open:

 			goto closefs;

 		}

 		if (io_ptr != io_ptr_orig) {

-			ext2fs_close(fs);

+			ext2fs_close_free(&fs);

 			goto retry_open;

 		}

 	}

@@ -2267,5 +2267,5 @@ closefs:

 		exit(1);

 	}

-	return (ext2fs_close(fs) ? 1 : 0);

+	return (ext2fs_close_free(&fs) ? 1 : 0);

 }

diff --git a/resize/main.c b/resize/main.c

index 80903b22..3951b091 100644

--- a/resize/main.c

+++ b/resize/main.c

@@ -484,7 +484,7 @@ int main (int argc, char ** argv)

 			_("Please run 'e2fsck -fy %s' to fix the filesystem\n"

 			  "after the aborted resize operation.\n"),

 			device_name);

-		ext2fs_close(fs);

+		ext2fs_close_free(&fs);

 		exit(1);

 	}

 	printf(_("The filesystem on %s is now %llu blocks long.\n\n"),

diff --git a/resize/resize2fs.c b/resize/resize2fs.c

index d6fc5337..a73390de 100644

--- a/resize/resize2fs.c

+++ b/resize/resize2fs.c

@@ -195,7 +195,7 @@ errcode_t resize_fs(ext2_filsys fs, blk64_t *new_size, int flags,

 	rfs->new_fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;

 	print_resource_track(rfs, &overall_track, fs->io);

-	retval = ext2fs_close(rfs->new_fs);

+	retval = ext2fs_close_free(&rfs->new_fs);

 	if (retval)

 		goto errout;

-- 

2.20.1