e2fsprogs: introduce ext2fs_close_free() helper

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>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/debian/e2fslibs.symbols b/debian/e2fslibs.symbols
index 5422544f9cfd541404d26b17f35fbe5998704539..1c769126743fba19d33db4f2b205203d52462764 100644 (file)
--- a/debian/e2fslibs.symbols
+++ b/debian/e2fslibs.symbols
@@ -111,6 +111,7 @@ libext2fs.so.2 e2fslibs #MINVER#
  ext2fs_clear_inode_bitmap@Base 1.37
  ext2fs_close2@Base 1.42
  ext2fs_close@Base 1.37
+ ext2fs_close_free@Base 1.42.11
  ext2fs_close_inode_scan@Base 1.37
  ext2fs_compare_block_bitmap@Base 1.37
  ext2fs_compare_generic_bitmap@Base 1.41.0

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index e53e0b927c705cc2867e320640bad74646db98d1..1703aba8f3618484cda0fd0a20ffcfb461d0c773 100644 (file)
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -132,10 +132,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)
@@ -241,10 +240,9 @@ static void close_filesystem(NOARGS)
        }
        if (current_qctx)
                quota_release_context(&current_qctx);
-       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 16380b313ace9fdf631fe79f83eb2aa59d2d3ebe..6131141003eef1e8e6d17e13cf840f60855917ab 100644 (file)
--- 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 3f3479c32ec1ac443d048540449f8a055b702f8f..b265c99ecee6f2a862c4b3e6786261850b34a2aa 100644 (file)
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -458,8 +458,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);
 }
@@ -1307,12 +1306,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,
@@ -1406,7 +1405,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,
@@ -1462,8 +1461,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;
        }
@@ -1513,8 +1511,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;
                }
@@ -1693,7 +1690,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) {
@@ -1775,8 +1772,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 434863137ce960afed7173354eb47b8652f5be54..fa529b4e09381e641af134a743afc2cd021e2389 100644 (file)
--- 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 4e91778ada00c9a8266ee095393bd7c94c92a16f..4599eef5d420ea4ce16fed6158e877a9c0c78f32 100644 (file)
--- 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 1491c622ba3782bfac28bb7ceb686f814b5c5a83..cbee97e845be25248715651079e793d788c7db21 100644 (file)
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -927,6 +927,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 884d9c07562121d855e59c740af1c6fa882e5b8c..2331e9ed3771304db4c325662dd268afd345ee44 100644 (file)
--- a/lib/ext2fs/mkjournal.c
+++ b/lib/ext2fs/mkjournal.c
@@ -642,7 +642,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 4e02adedc50171e67c6ba86e124fc3e801ea9504..574fb7a7fae39018235c6660aac30cf09901fcfa 100644 (file)
--- 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 d4bde8e5104bcea704ad015a52cc9ac59d2ac027..cc18ad835d66b852f505e14d37d8b618b2799c10 100644 (file)
--- 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 612ca445ad8a37427f33e15fd90ce6ec7d982317..bb72c70d4606e2c9377af0f18d802d337f42ae4f 100644 (file)
--- 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 338d239c78359b3189d70872a2506b62b541a881..82fe3e8a05a2988211cc30451a8dca33568a6795 100644 (file)
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -1428,7 +1428,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)
@@ -1662,7 +1662,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 0db5e4bdd4d7d1e110881228033aae246f75a782..a2047935dd91a9ffb8445d7d10d0cdf3a1f87b96 100644 (file)
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -1808,7 +1808,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 883b8f2cb2910c2de9387965630f6ec4e46099e6..eebfba9ed09be6ae7cc3db830c475a24d6e148d1 100644 (file)
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1751,7 +1751,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) {
@@ -2708,7 +2708,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)
@@ -2830,7 +2830,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 &
@@ -2894,7 +2894,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."));

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 9ba32a1c1a381677798af098bac5352dc3816c79..6b2123518240c69bbee42202b9a959b07a59ae68 100644 (file)
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -698,7 +698,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"),
@@ -2001,7 +2001,7 @@ retry_open:
                        goto closefs;
                }
                if (io_ptr != io_ptr_orig) {
-                       ext2fs_close(fs);
+                       ext2fs_close_free(&fs);
                        goto retry_open;
                }
        }
@@ -2289,5 +2289,5 @@ closefs:
                exit(1);
        }
 
-       return (ext2fs_close(fs) ? 1 : 0);
+       return (ext2fs_close_free(&fs) ? 1 : 0);
 }

diff --git a/misc/util.c b/misc/util.c
index 8d02ab22a94ce3c26763d69e99f6097ec6aef622..1fcae1d54aef3762547b9a8d48fcf17afcca462c 100644 (file)
--- a/misc/util.c
+++ b/misc/util.c
@@ -140,7 +140,7 @@ static void print_ext2_info(const char *device)
                tm = sb->s_wtime;
                printf(_("\tlast modified on %s"), ctime(&tm));
        }
-       ext2fs_close(fs);
+       ext2fs_close_free(&fs);
 }
 
 /*

diff --git a/resize/main.c b/resize/main.c
index e65c8e4de6819b8b789baf9d8c9f97a3ab0284da..e4b44354d121476cff6e0540dd53af4742b2759d 100644 (file)
--- a/resize/main.c
+++ b/resize/main.c
@@ -464,7 +464,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 e528f6274014053a8f704fecff3a3ad4075f3475..6bd2e1c62f7b7a6daddacc10fea50773e78e63f3 100644 (file)
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -202,7 +202,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;