Hi Andreas, thank you for your input! > Having just looked through the patch, I think it could use some cleanup. > Basic code style issues: > - wrapping lines at 80-columns > - avoid use of C++ comments in the code > - consistent indentation for continued lines > - consistent one blank line between functions > - consistent one blank line after variable declarations I've used clang-format with the `.clang-format` from the linux git to format the code. I also ran `scripts/checkpatch.pl` from linux to fix some more issues. > - split large highly-indented code blocks into helper functions There was particularly one highly indented switch() which I now put into its own function. > In terms of code structure, refactoring it to put libarchive handling > in a separate file (e.g. mke2fs-archive.c or similar) would also make > the maintenance easier, since it can be added/removed from the build > more easily, and (if necessary) removed from the tree if it is no longer > working. I've moved most of the libarchive functions into misc/create_inode_libarchive.c and hope this improves the situation. I've had to modify a couple of Makefile.in, make some functions of misc/create_inode.c non-extern and add them to create_inode.h so that misc/create_inode_libarchive.c can make use of them. Is this what you had in mind? > Then have only a couple of small function calls in the main mke2fs.c > code that are accessing the libarchive functionality if it is built-in, > or being no-ops (or just printing the error message) if libarchive is > unavailable. All the libarchive-specific functionality is behind __populate_fs_from_tar() which is the only function exposed in misc/create_inode_libarchive.h. If e2fsprogs was compiled without libarchive, an error message will be shown if the user tries to pass a regular file instead of a directory. If e2fsprogs was compiled with libarchive but the user does not have the shared library installed, another error about this will be displayed. There are probably still many things about my patch that can be improved. Thanks a lot for considering this and having a look at it! cheers, josch P.S. (more like a record for future-me) I tested that my changes allow libarchive to be compiled with and without libarchive headers installed and with and without libarchive shared library installed by running: mmdebstrap --variant=apt --include=git,ca-certificates,build-essential,autoconf,automake,autoconf-archive,pkg-config,gettext,texinfo,libblkid-dev,uuid-dev,m4,libarchive-dev \ --chrooted-customize-hook='git clone https://github.com/josch/e2fsprogs.git && cd e2fsprogs && git checkout libarchive-linux-ext4 && autoreconf -fi && ./configure && make -j4 && make check || cat tests/m_rootgnutar.failed tests/m_rootgnutar.log && apt remove --yes libarchive13 && tar -C include -c . | ./misc/mke2fs -q -F -o Linux -T ext4 -O metadata_csum,64bit -E lazy_itable_init=1 -b 1024 -d - image.ext4 16384' \ unstable /dev/null Johannes Schauer Marin Rodrigues (1): mke2fs: the -d option can now handle tarball input MCONFIG.in | 1 + configure | 134 ++++--- configure.ac | 9 + debugfs/Makefile.in | 25 +- lib/config.h.in | 3 + lib/ext2fs/Makefile.in | 25 +- misc/Makefile.in | 17 +- misc/create_inode.c | 61 ++- misc/create_inode.h | 10 + misc/create_inode_libarchive.c | 677 +++++++++++++++++++++++++++++++++ misc/create_inode_libarchive.h | 10 + misc/mke2fs.8.in | 10 +- misc/mke2fs.c | 12 +- tests/m_rootgnutar/expect | 141 +++++++ tests/m_rootgnutar/output.sed | 5 + tests/m_rootgnutar/script | 123 ++++++ tests/m_rootpaxtar/expect | 87 +++++ tests/m_rootpaxtar/mkpaxtar.pl | 69 ++++ tests/m_rootpaxtar/output.sed | 5 + tests/m_rootpaxtar/script | 44 +++ tests/m_roottar/expect | 208 ++++++++++ tests/m_roottar/mktar.pl | 62 +++ tests/m_roottar/output.sed | 5 + tests/m_roottar/script | 57 +++ 24 files changed, 1714 insertions(+), 86 deletions(-) create mode 100644 misc/create_inode_libarchive.c create mode 100644 misc/create_inode_libarchive.h create mode 100644 tests/m_rootgnutar/expect create mode 100644 tests/m_rootgnutar/output.sed create mode 100644 tests/m_rootgnutar/script create mode 100644 tests/m_rootpaxtar/expect create mode 100644 tests/m_rootpaxtar/mkpaxtar.pl create mode 100644 tests/m_rootpaxtar/output.sed create mode 100644 tests/m_rootpaxtar/script create mode 100644 tests/m_roottar/expect create mode 100644 tests/m_roottar/mktar.pl create mode 100644 tests/m_roottar/output.sed create mode 100644 tests/m_roottar/script -- 2.40.0
