Skip to content

Commit 47fee2e

Browse files
Lukas Czernertytso
Lukas Czerner
authored and
tytso
committed
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>
1 parent e7822c1 commit 47fee2e

18 files changed

+48
-44
lines changed

‎debian/e2fslibs.symbols

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ libext2fs.so.2 e2fslibs #MINVER#
111111
ext2fs_clear_inode_bitmap@Base 1.37
112112
ext2fs_close2@Base 1.42
113113
ext2fs_close@Base 1.37
114+
ext2fs_close_free@Base 1.42.11
114115
ext2fs_close_inode_scan@Base 1.37
115116
ext2fs_compare_block_bitmap@Base 1.37
116117
ext2fs_compare_generic_bitmap@Base 1.41.0

‎debugfs/debugfs.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,9 @@ static void open_filesystem(char *device, int open_flags, blk64_t superblock,
132132
return;
133133

134134
errout:
135-
retval = ext2fs_close(current_fs);
135+
retval = ext2fs_close_free(&current_fs);
136136
if (retval)
137137
com_err(device, retval, "while trying to close filesystem");
138-
current_fs = NULL;
139138
}
140139

141140
void do_open_filesys(int argc, char **argv)
@@ -241,10 +240,9 @@ static void close_filesystem(NOARGS)
241240
}
242241
if (current_qctx)
243242
quota_release_context(&current_qctx);
244-
retval = ext2fs_close(current_fs);
243+
retval = ext2fs_close_free(&current_fs);
245244
if (retval)
246245
com_err("ext2fs_close", retval, 0);
247-
current_fs = NULL;
248246
return;
249247
}
250248

‎e2fsck/scantest.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ int main (int argc, char *argv[])
133133
}
134134

135135

136-
ext2fs_close(fs);
136+
ext2fs_close_free(&fs);
137137

138138
print_resource_track(&global_rtrack);
139139

‎e2fsck/unix.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,7 @@ static void check_if_skip(e2fsck_t ctx)
458458
}
459459
log_out(ctx, "\n");
460460
skip:
461-
ext2fs_close(fs);
462-
ctx->fs = NULL;
461+
ext2fs_close_free(&fs);
463462
e2fsck_free_context(ctx);
464463
exit(FSCK_OK);
465464
}
@@ -1307,12 +1306,12 @@ int main (int argc, char *argv[])
13071306
orig_superblock = ctx->superblock;
13081307
get_backup_sb(ctx, fs, ctx->filesystem_name, io_ptr);
13091308
if (fs)
1310-
ext2fs_close(fs);
1309+
ext2fs_close_free(&fs);
13111310
orig_retval = retval;
13121311
retval = try_open_fs(ctx, flags, io_ptr, &fs);
13131312
if ((orig_retval == 0) && retval != 0) {
13141313
if (fs)
1315-
ext2fs_close(fs);
1314+
ext2fs_close_free(&fs);
13161315
log_out(ctx, _("%s: %s while using the "
13171316
"backup blocks"),
13181317
ctx->program_name,
@@ -1406,7 +1405,7 @@ int main (int argc, char *argv[])
14061405
* reopen the filesystem after we get the device size.
14071406
*/
14081407
if (pctx.errcode == EBUSY) {
1409-
ext2fs_close(fs);
1408+
ext2fs_close_free(&fs);
14101409
need_restart++;
14111410
pctx.errcode =
14121411
ext2fs_get_device_size2(ctx->filesystem_name,
@@ -1462,8 +1461,7 @@ int main (int argc, char *argv[])
14621461
/*
14631462
* Restart in order to reopen fs but this time start mmp.
14641463
*/
1465-
ext2fs_close(fs);
1466-
ctx->fs = NULL;
1464+
ext2fs_close_free(&fs);
14671465
flags &= ~EXT2_FLAG_SKIP_MMP;
14681466
goto restart;
14691467
}
@@ -1513,8 +1511,7 @@ int main (int argc, char *argv[])
15131511
ctx->device_name);
15141512
fatal_error(ctx, 0);
15151513
}
1516-
ext2fs_close(ctx->fs);
1517-
ctx->fs = 0;
1514+
ext2fs_close_free(&ctx->fs);
15181515
ctx->flags |= E2F_FLAG_RESTARTED;
15191516
goto restart;
15201517
}
@@ -1693,7 +1690,7 @@ int main (int argc, char *argv[])
16931690
_("while resetting context"));
16941691
fatal_error(ctx, 0);
16951692
}
1696-
ext2fs_close(fs);
1693+
ext2fs_close_free(&fs);
16971694
goto restart;
16981695
}
16991696
if (run_result & E2F_FLAG_CANCEL) {
@@ -1775,8 +1772,7 @@ int main (int argc, char *argv[])
17751772
io_channel_flush(ctx->fs->io);
17761773
print_resource_track(ctx, NULL, &ctx->global_rtrack, ctx->fs->io);
17771774

1778-
ext2fs_close(fs);
1779-
ctx->fs = NULL;
1775+
ext2fs_close_free(&fs);
17801776
free(ctx->journal_name);
17811777

17821778
e2fsck_free_context(ctx);

‎e2fsck/util.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ void preenhalt(e2fsck_t ctx)
319319
if (fs != NULL) {
320320
fs->super->s_state |= EXT2_ERROR_FS;
321321
ext2fs_mark_super_dirty(fs);
322-
ext2fs_close(fs);
322+
ext2fs_close_free(&fs);
323323
}
324324
exit(FSCK_UNCORRECTED);
325325
}

‎lib/ext2fs/closefs.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,18 @@ errcode_t ext2fs_flush2(ext2_filsys fs, int flags)
437437
return retval;
438438
}
439439

440+
errcode_t ext2fs_close_free(ext2_filsys *fs_ptr)
441+
{
442+
errcode_t ret;
443+
ext2_filsys fs = *fs_ptr;
444+
445+
ret = ext2fs_close2(fs, 0);
446+
if (ret)
447+
ext2fs_free(fs);
448+
*fs_ptr = NULL;
449+
return ret;
450+
}
451+
440452
errcode_t ext2fs_close(ext2_filsys fs)
441453
{
442454
return ext2fs_close2(fs, 0);

‎lib/ext2fs/ext2fs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,7 @@ extern errcode_t ext2fs_check_desc(ext2_filsys fs);
927927
/* closefs.c */
928928
extern errcode_t ext2fs_close(ext2_filsys fs);
929929
extern errcode_t ext2fs_close2(ext2_filsys fs, int flags);
930+
extern errcode_t ext2fs_close_free(ext2_filsys *fs);
930931
extern errcode_t ext2fs_flush(ext2_filsys fs);
931932
extern errcode_t ext2fs_flush2(ext2_filsys fs, int flags);
932933
extern int ext2fs_bg_has_super(ext2_filsys fs, dgrp_t group_block);

‎lib/ext2fs/mkjournal.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ main(int argc, char **argv)
642642
if (retval) {
643643
printf("Warning, had trouble writing out superblocks.\n");
644644
}
645-
ext2fs_close(fs);
645+
ext2fs_close_free(&fs);
646646
exit(0);
647647

648648
}

‎lib/ext2fs/tst_bitmaps.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,7 @@ static void setup_filesystem(const char *name,
187187
return;
188188

189189
errout:
190-
ext2fs_close(test_fs);
191-
test_fs = 0;
190+
ext2fs_close_free(&test_fs);
192191
}
193192

194193
void setup_cmd(int argc, char **argv)
@@ -199,10 +198,8 @@ void setup_cmd(int argc, char **argv)
199198
unsigned int type = EXT2FS_BMAP64_BITARRAY;
200199
int flags = EXT2_FLAG_64BITS;
201200

202-
if (test_fs) {
203-
ext2fs_close(test_fs);
204-
test_fs = 0;
205-
}
201+
if (test_fs)
202+
ext2fs_close_free(&test_fs);
206203

207204
reset_getopt();
208205
while ((c = getopt(argc, argv, "b:i:lt:")) != EOF) {
@@ -242,8 +239,7 @@ void close_cmd(int argc, char **argv)
242239
if (check_fs_open(argv[0]))
243240
return;
244241

245-
ext2fs_close(test_fs);
246-
test_fs = 0;
242+
ext2fs_close_free(&test_fs);
247243
}
248244

249245

‎misc/dumpe2fs.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ int main (int argc, char ** argv)
614614
if (fs->super->s_feature_incompat &
615615
EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {
616616
print_journal_information(fs);
617-
ext2fs_close(fs);
617+
ext2fs_close_free(&fs);
618618
exit(0);
619619
}
620620
if ((fs->super->s_feature_compat &
@@ -623,7 +623,7 @@ int main (int argc, char ** argv)
623623
print_inline_journal_information(fs);
624624
list_bad_blocks(fs, 0);
625625
if (header_only) {
626-
ext2fs_close (fs);
626+
ext2fs_close_free(&fs);
627627
exit (0);
628628
}
629629
retval = ext2fs_read_bitmaps (fs);
@@ -634,7 +634,7 @@ int main (int argc, char ** argv)
634634
error_message(retval));
635635
}
636636
}
637-
ext2fs_close (fs);
637+
ext2fs_close_free(&fs);
638638
remove_error_table(&et_ext2_error_table);
639639
exit (0);
640640
}

‎misc/e2freefrag.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ static errcode_t get_chunk_info(ext2_filsys fs, struct chunk_info *info,
215215

216216
static void close_device(char *device_name, ext2_filsys fs)
217217
{
218-
int retval = ext2fs_close(fs);
218+
int retval = ext2fs_close_free(&fs);
219219

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

‎misc/e2image.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ static void install_image(char *device, char *image_fn, int type)
14281428
}
14291429

14301430
close(fd);
1431-
ext2fs_close (fs);
1431+
ext2fs_close_free(&fs);
14321432
}
14331433

14341434
static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)
@@ -1662,7 +1662,7 @@ int main (int argc, char ** argv)
16621662
else
16631663
write_image_file(fs, fd);
16641664

1665-
ext2fs_close (fs);
1665+
ext2fs_close_free(&fs);
16661666
if (check)
16671667
printf(_("%d blocks already contained the data to be copied\n"),
16681668
skipped_blocks);

‎misc/e4defrag.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1808,7 +1808,7 @@ int main(int argc, char *argv[])
18081808
feature_incompat = fs->super->s_feature_incompat;
18091809
log_groups_per_flex = fs->super->s_log_groups_per_flex;
18101810

1811-
ext2fs_close(fs);
1811+
ext2fs_close_free(&fs);
18121812
}
18131813

18141814
switch (arg_type) {

‎misc/mke2fs.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ static void PRS(int argc, char *argv[])
17511751
printf(_("Using journal device's blocksize: %d\n"), blocksize);
17521752
fs_param.s_log_block_size =
17531753
int_log2(blocksize >> EXT2_MIN_BLOCK_LOG_SIZE);
1754-
ext2fs_close(jfs);
1754+
ext2fs_close_free(&jfs);
17551755
}
17561756

17571757
if (optind < argc) {
@@ -2708,7 +2708,7 @@ int main (int argc, char *argv[])
27082708
if (fs->super->s_feature_incompat &
27092709
EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {
27102710
create_journal_dev(fs);
2711-
exit(ext2fs_close(fs) ? 1 : 0);
2711+
exit(ext2fs_close_free(&fs) ? 1 : 0);
27122712
}
27132713

27142714
if (bad_blocks_filename)
@@ -2830,7 +2830,7 @@ int main (int argc, char *argv[])
28302830
}
28312831
if (!quiet)
28322832
printf("%s", _("done\n"));
2833-
ext2fs_close(jfs);
2833+
ext2fs_close_free(&jfs);
28342834
free(journal_device);
28352835
} else if ((journal_size) ||
28362836
(fs_param.s_feature_compat &
@@ -2894,7 +2894,7 @@ int main (int argc, char *argv[])
28942894
"filesystem accounting information: "));
28952895
checkinterval = fs->super->s_checkinterval;
28962896
max_mnt_count = fs->super->s_max_mnt_count;
2897-
retval = ext2fs_close(fs);
2897+
retval = ext2fs_close_free(&fs);
28982898
if (retval) {
28992899
fprintf(stderr, "%s",
29002900
_("\nWarning, had trouble writing out superblocks."));

‎misc/tune2fs.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ static int add_journal(ext2_filsys fs)
698698
fflush(stdout);
699699

700700
retval = ext2fs_add_journal_device(fs, jfs);
701-
ext2fs_close(jfs);
701+
ext2fs_close_free(&jfs);
702702
if (retval) {
703703
com_err(program_name, retval,
704704
_("while adding filesystem to journal on %s"),
@@ -2001,7 +2001,7 @@ int main(int argc, char **argv)
20012001
goto closefs;
20022002
}
20032003
if (io_ptr != io_ptr_orig) {
2004-
ext2fs_close(fs);
2004+
ext2fs_close_free(&fs);
20052005
goto retry_open;
20062006
}
20072007
}
@@ -2289,5 +2289,5 @@ int main(int argc, char **argv)
22892289
exit(1);
22902290
}
22912291

2292-
return (ext2fs_close(fs) ? 1 : 0);
2292+
return (ext2fs_close_free(&fs) ? 1 : 0);
22932293
}

‎misc/util.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ static void print_ext2_info(const char *device)
140140
tm = sb->s_wtime;
141141
printf(_("\tlast modified on %s"), ctime(&tm));
142142
}
143-
ext2fs_close(fs);
143+
ext2fs_close_free(&fs);
144144
}
145145

146146
/*

‎resize/main.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ int main (int argc, char ** argv)
464464
_("Please run 'e2fsck -fy %s' to fix the filesystem\n"
465465
"after the aborted resize operation.\n"),
466466
device_name);
467-
ext2fs_close(fs);
467+
ext2fs_close_free(&fs);
468468
exit(1);
469469
}
470470
printf(_("The filesystem on %s is now %llu blocks long.\n\n"),

‎resize/resize2fs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ errcode_t resize_fs(ext2_filsys fs, blk64_t *new_size, int flags,
202202
rfs->new_fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
203203

204204
print_resource_track(rfs, &overall_track, fs->io);
205-
retval = ext2fs_close(rfs->new_fs);
205+
retval = ext2fs_close_free(&rfs->new_fs);
206206
if (retval)
207207
goto errout;
208208

0 commit comments

Comments
 (0)