Message ID | 20180525124225.31089-2-artem.blagodarenko@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | e2image -b option to pass superblock number | expand |
On May 25, 2018, at 6:42 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote: > > There are similar opening filesystem code in different utilities. > > The patch moves this code to ext2fs_try_open_fs(). I think the patch is fine, but this comment is not consistent with the actual patch. It is moving the improved handling *from* try_open_fs() into ext2fs_open(). > This function make one of the action based on parameters: > 1) open filesystem with given superblock, superblock size > 2) open filesystem with given superblock, but try to > find right block size > 3) open filesystem with default superblock and block size > > Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com> Aside from minor whitespace and comment cleanup, you can add: Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > e2fsck/unix.c | 28 +++------------------------- > lib/ext2fs/openfs.c | 36 +++++++++++++++++++++++++++++++++++- > misc/dumpe2fs.c | 17 +++-------------- > 3 files changed, 41 insertions(+), 40 deletions(-) > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c > index cbe5ec50..491e9eb6 100644 > --- a/e2fsck/unix.c > +++ b/e2fsck/unix.c > @@ -1142,31 +1142,9 @@ static errcode_t try_open_fs(e2fsck_t ctx, int flags, io_manager io_ptr, > ext2_filsys *ret_fs) > { > errcode_t retval; > - > - *ret_fs = NULL; > - if (ctx->superblock && ctx->blocksize) { > - retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, > - flags, ctx->superblock, ctx->blocksize, > - io_ptr, ret_fs); > - } else if (ctx->superblock) { > - int blocksize; > - for (blocksize = EXT2_MIN_BLOCK_SIZE; > - blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) { > - if (*ret_fs) { > - ext2fs_free(*ret_fs); > - *ret_fs = NULL; > - } > - retval = ext2fs_open2(ctx->filesystem_name, > - ctx->io_options, flags, > - ctx->superblock, blocksize, > - io_ptr, ret_fs); > - if (!retval) > - break; > - } > - } else > - retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, > - flags, 0, 0, io_ptr, ret_fs); > - > + retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, > + flags, ctx->superblock, ctx->blocksize, > + io_ptr, ret_fs); > if (retval == 0) { > (*ret_fs)->priv_data = ctx; > e2fsck_set_bitmap_type(*ret_fs, EXT2FS_BMAP64_RBTREE, > diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c > index 532e70f7..d18e1ad2 100644 > --- a/lib/ext2fs/openfs.c > +++ b/lib/ext2fs/openfs.c > @@ -108,7 +108,7 @@ errcode_t ext2fs_open(const char *name, int flags, int superblock, > * EXT2_FLAG_64BITS - Allow 64-bit bitfields (needed for large > * filesystems) > */ > -errcode_t ext2fs_open2(const char *name, const char *io_options, > +static errcode_t __ext2fs_open2(const char *name, const char *io_options, > int flags, int superblock, > unsigned int block_size, io_manager manager, > ext2_filsys *ret_fs) > @@ -504,6 +504,40 @@ cleanup: > return retval; > } > > +errcode_t ext2fs_open2(const char *name, const char *io_options, > + int flags, int superblock, > + unsigned int block_size, io_manager manager, > + ext2_filsys *ret_fs) > +{ > + errcode_t retval; > + > + *ret_fs = NULL; > + if (superblock && block_size) { > + retval = __ext2fs_open2(name, io_options, > + flags, superblock, block_size, (style) align after '(' on previous line > + manager, ret_fs); > + } else if (superblock) { > + int block_size; > + > + for (block_size = EXT2_MIN_BLOCK_SIZE; > + block_size <= EXT2_MAX_BLOCK_SIZE; block_size *= 2) { > + if (*ret_fs) { > + ext2fs_free(*ret_fs); > + *ret_fs = NULL; > + } > + retval = __ext2fs_open2(name, > + io_options, flags, (style) align after '(' on previous line > + superblock, block_size, > + manager, ret_fs); > + if (!retval) > + break; > + } > + } else > + retval = __ext2fs_open2(name, io_options, > + flags, 0, 0, manager, ret_fs); (style) align after '(' on previous line (style) consistent {} around all branches of if/else block > + return retval; > +} > + > /* > * Set/get the filesystem data I/O channel. > * > diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c > index 395ea9ee..6c4d7965 100644 > --- a/misc/dumpe2fs.c > +++ b/misc/dumpe2fs.c > @@ -568,20 +568,9 @@ int main (int argc, char ** argv) > if (image_dump) > flags |= EXT2_FLAG_IMAGE_FILE; > try_open_again: > - if (use_superblock && !use_blocksize) { > - for (use_blocksize = EXT2_MIN_BLOCK_SIZE; > - use_blocksize <= EXT2_MAX_BLOCK_SIZE; > - use_blocksize *= 2) { > - retval = ext2fs_open (device_name, flags, > - use_superblock, > - use_blocksize, unix_io_manager, > - &fs); > - if (!retval) > - break; > - } > - } else > - retval = ext2fs_open (device_name, flags, use_superblock, > - use_blocksize, unix_io_manager, &fs); > + retval = ext2fs_open2(device_name, 0, > + flags, use_superblock, use_blocksize, > + unix_io_manager, &fs); > if (retval && !(flags & EXT2_FLAG_IGNORE_CSUM_ERRORS)) { > flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS; > goto try_open_again; > -- > 2.14.3 > Cheers, Andreas
diff --git a/e2fsck/unix.c b/e2fsck/unix.c index cbe5ec50..491e9eb6 100644 --- a/e2fsck/unix.c +++ b/e2fsck/unix.c @@ -1142,31 +1142,9 @@ static errcode_t try_open_fs(e2fsck_t ctx, int flags, io_manager io_ptr, ext2_filsys *ret_fs) { errcode_t retval; - - *ret_fs = NULL; - if (ctx->superblock && ctx->blocksize) { - retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, - flags, ctx->superblock, ctx->blocksize, - io_ptr, ret_fs); - } else if (ctx->superblock) { - int blocksize; - for (blocksize = EXT2_MIN_BLOCK_SIZE; - blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) { - if (*ret_fs) { - ext2fs_free(*ret_fs); - *ret_fs = NULL; - } - retval = ext2fs_open2(ctx->filesystem_name, - ctx->io_options, flags, - ctx->superblock, blocksize, - io_ptr, ret_fs); - if (!retval) - break; - } - } else - retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, - flags, 0, 0, io_ptr, ret_fs); - + retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options, + flags, ctx->superblock, ctx->blocksize, + io_ptr, ret_fs); if (retval == 0) { (*ret_fs)->priv_data = ctx; e2fsck_set_bitmap_type(*ret_fs, EXT2FS_BMAP64_RBTREE, diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c index 532e70f7..d18e1ad2 100644 --- a/lib/ext2fs/openfs.c +++ b/lib/ext2fs/openfs.c @@ -108,7 +108,7 @@ errcode_t ext2fs_open(const char *name, int flags, int superblock, * EXT2_FLAG_64BITS - Allow 64-bit bitfields (needed for large * filesystems) */ -errcode_t ext2fs_open2(const char *name, const char *io_options, +static errcode_t __ext2fs_open2(const char *name, const char *io_options, int flags, int superblock, unsigned int block_size, io_manager manager, ext2_filsys *ret_fs) @@ -504,6 +504,40 @@ cleanup: return retval; } +errcode_t ext2fs_open2(const char *name, const char *io_options, + int flags, int superblock, + unsigned int block_size, io_manager manager, + ext2_filsys *ret_fs) +{ + errcode_t retval; + + *ret_fs = NULL; + if (superblock && block_size) { + retval = __ext2fs_open2(name, io_options, + flags, superblock, block_size, + manager, ret_fs); + } else if (superblock) { + int block_size; + + for (block_size = EXT2_MIN_BLOCK_SIZE; + block_size <= EXT2_MAX_BLOCK_SIZE; block_size *= 2) { + if (*ret_fs) { + ext2fs_free(*ret_fs); + *ret_fs = NULL; + } + retval = __ext2fs_open2(name, + io_options, flags, + superblock, block_size, + manager, ret_fs); + if (!retval) + break; + } + } else + retval = __ext2fs_open2(name, io_options, + flags, 0, 0, manager, ret_fs); + return retval; +} + /* * Set/get the filesystem data I/O channel. * diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c index 395ea9ee..6c4d7965 100644 --- a/misc/dumpe2fs.c +++ b/misc/dumpe2fs.c @@ -568,20 +568,9 @@ int main (int argc, char ** argv) if (image_dump) flags |= EXT2_FLAG_IMAGE_FILE; try_open_again: - if (use_superblock && !use_blocksize) { - for (use_blocksize = EXT2_MIN_BLOCK_SIZE; - use_blocksize <= EXT2_MAX_BLOCK_SIZE; - use_blocksize *= 2) { - retval = ext2fs_open (device_name, flags, - use_superblock, - use_blocksize, unix_io_manager, - &fs); - if (!retval) - break; - } - } else - retval = ext2fs_open (device_name, flags, use_superblock, - use_blocksize, unix_io_manager, &fs); + retval = ext2fs_open2(device_name, 0, + flags, use_superblock, use_blocksize, + unix_io_manager, &fs); if (retval && !(flags & EXT2_FLAG_IGNORE_CSUM_ERRORS)) { flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS; goto try_open_again;
There are similar opening filesystem code in different utilities. The patch moves this code to ext2fs_try_open_fs(). This function make one of the action based on parameters: 1) open filesystem with given superblock, superblock size 2) open filesystem with given superblock, but try to find right block size 3) open filesystem with default superblock and block size Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com> --- e2fsck/unix.c | 28 +++------------------------- lib/ext2fs/openfs.c | 36 +++++++++++++++++++++++++++++++++++- misc/dumpe2fs.c | 17 +++-------------- 3 files changed, 41 insertions(+), 40 deletions(-)