by Andreas Dilger

[permalink] [raw]

Subject: Re: [PATCH 07/38] lib/blkid: remove 32-bit x86 byteswap assembly

On Jan 21, 2023, at 1:31 PM, Eric Biggers <[email protected]> wrote:
>
> From: Eric Biggers <[email protected]>
>
> libblkid contains 32-bit x86 assembly language implementations of 16-bit
> and 32-bit byteswaps. However, modern compilers can easily generate the
> bswap instruction automatically from the corresponding C expression.
> And no one ever bothered to add assembly for x86_64 or other
> architectures, anyway. So let's just remove this outdated code, which
> was maybe useful in the 90s, but is no longer useful.

I'm not sure what Ted thinks about this, but it might be time to remove
the libblkid implementation in e2fsprogs, and instead depend on the
newer version in util-linux? However, removing it from e2fsprogs
would add an external dependency on util-linux for non-linux platforms,
though AFAIK it should still be possible to build e2fsprogs without it.

That said, I think the current patch looks fine, and you can add:

Reviewed-by: Andreas Dilger <[email protected]>

Cheers, Andreas

>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> lib/blkid/probe.h | 43 -------------------------------------------
> 1 file changed, 43 deletions(-)
>
> diff --git a/lib/blkid/probe.h b/lib/blkid/probe.h
> index dea4081d0..063a5b5c0 100644
> --- a/lib/blkid/probe.h
> +++ b/lib/blkid/probe.h
> @@ -814,46 +814,6 @@ struct exfat_entry_label {
> #define _INLINE_ static inline
> #endif
>
> -static __u16 blkid_swab16(__u16 val);
> -static __u32 blkid_swab32(__u32 val);
> -static __u64 blkid_swab64(__u64 val);
> -
> -#if ((defined __GNUC__) && \
> - (defined(__i386__) || defined(__i486__) || defined(__i586__)))
> -
> -#define _BLKID_HAVE_ASM_BITOPS_
> -
> -_INLINE_ __u32 blkid_swab32(__u32 val)
> -{
> -#ifdef EXT2FS_REQUIRE_486
> - __asm__("bswap %0" : "=r" (val) : "0" (val));
> -#else
> - __asm__("xchgb %b0,%h0\n\t" /* swap lower bytes */
> - "rorl $16,%0\n\t" /* swap words */
> - "xchgb %b0,%h0" /* swap higher bytes */
> - :"=q" (val)
> - : "0" (val));
> -#endif
> - return val;
> -}
> -
> -_INLINE_ __u16 blkid_swab16(__u16 val)
> -{
> - __asm__("xchgb %b0,%h0" /* swap bytes */ \
> - : "=q" (val) \
> - : "0" (val)); \
> - return val;
> -}
> -
> -_INLINE_ __u64 blkid_swab64(__u64 val)
> -{
> - return (blkid_swab32(val >> 32) |
> - (((__u64) blkid_swab32(val & 0xFFFFFFFFUL)) << 32));
> -}
> -#endif
> -
> -#if !defined(_BLKID_HAVE_ASM_BITOPS_)
> -
> _INLINE_ __u16 blkid_swab16(__u16 val)
> {
> return (val >> 8) | (val << 8);
> @@ -870,9 +830,6 @@ _INLINE_ __u64 blkid_swab64(__u64 val)
> return (blkid_swab32(val >> 32) |
> (((__u64) blkid_swab32(val & 0xFFFFFFFFUL)) << 32));
> }
> -#endif
> -
> -
>
> #ifdef WORDS_BIGENDIAN
> #define blkid_le16(x) blkid_swab16(x)
> --
> 2.39.0
>

Cheers, Andreas

Attachments:

signature.asc (873.00 B)
Message signed with OpenPGP

2023-01-24 20:59:24

by Andreas Dilger

[permalink] [raw]

Subject: Re: [PATCH 00/38] e2fsprogs: misc fixes, and add a GitHub Actions file

On Jan 21, 2023, at 1:31 PM, Eric Biggers <[email protected]> wrote:
>
> The last patch of this series adds a workflow file for GitHub Actions
> that builds and tests e2fsprogs on Ubuntu, macOS, and Windows. It's
> enforced that the build does not produce warnings with -Wall.
>
> (For now, the Windows build is much more basic than the others; only
> mke2fs is built, and the unit tests are not run.)
>
> The workflow will run on pushes to any fork of e2fsprogs that has GitHub
> Actions enabled. I'm hoping that Ted will enable it for the "official"
> fork at https://github.com/tytso/e2fsprogs, but anyone can use it in
> their own fork too. The results for this patch series are at
> https://github.com/ebiggers/e2fsprogs/actions/runs/3976382057
>
> As a prerequisite to actually getting everything to pass, patches 1-37
> of this series fix a large number of miscellaneous issues, mainly
> pertaining to warnings with -Wall or to the Windows build.

Thanks in advance for this patch series. I have a kick at the cat for
e2fsprogs build/test warnings occasionally, but getting this into a
Github CI setup should keep them out of the woodwork.

I'll try to have a pass through all of the patches, but most of them
seem pretty straight forward.

Cheers, Andreas

> Some patches in this series I've already sent out individually. This
> series supersedes all my previous patches.
>
> Eric Biggers (38):
> configure.ac: only use Windows I/O manager on native Windows
> configure.ac: disable tdb by default on Windows
> configure.ac: automatically add include/mingw/ headers
> configure: regenerate
> config/install-sh: update to latest version
> lib, misc: eliminate dependency on Winsock
> lib/blkid: remove 32-bit x86 byteswap assembly
> lib/blkid: fix unaligned access to hfs_mdb
> lib/blkid: fix -Wunused-variable warning in blkid_get_dev_size()
> lib/blkid: suppress -Wunused-result warning in blkid_flush_cache()
> lib/blkid: suppress -Wstringop-truncation warning in blkid_strndup()
> lib/e2p: fix a -Wunused-variable warning in getflags()
> lib/{e2p,ss}: remove manual declarations of errno
> lib/et: fix "unused variable" warnings when !HAVE_FCNTL
> lib/ext2fs: remove 32-bit x86 bitops assembly
> lib/ext2fs: consistently use #ifdefs in ext2fs_print_bmap_statistics()
> lib/ext2fs: remove unused variable in ext2fs_xattrs_read_inode()
> lib/ext2fs: fix a printf format specifier in file_test()
> lib/ext2fs: fix two compiler warnings in windows_io.c
> lib/ext2fs: fix a -Wpointer-sign warning in ext2fs_mmp_start()
> lib/{ext2fs,support}: fix 32-bit Windows build
> lib/ss: fix 'make install' by creating man1dir
> lib/support: remove unused label in get_devname()
> lib/support: clean up definition of flags_array
> lib/uuid: remove conflicting Windows implementation of gettimeofday()
> e2fsck: use real functions for kernel slab functions
> misc/create_inode: fix -Wunused-variable warnings in __populate_fs()
> misc/create_inode: simplify logic in scandir()
> misc/e4defrag: fix -Wstringop-truncation warnings
> misc/fuse2fs: avoid error-prone strncpy() pattern
> misc/mk_hugefiles: simplify get_partition_start()
> misc/mke2fs: fix Windows build
> misc/mke2fs: fix a -Wunused-variable warning in PRS()
> misc/tune2fs: fix setting fsuuid::fsu_len
> misc/tune2fs: fix -Wunused-variable warnings in handle_fslabel()
> misc/util.c: enable MinGW alarm() when building for Windows
> resize2fs: remove unused variable from adjust_superblock()
> Add a configuration file for GitHub Actions
>
> .github/workflows/ci.yml | 116 ++++++
> aclocal.m4 | 180 +++++----
> config/install-sh | 683 ++++++++++++++++++++++++----------
> configure | 105 ++++--
> configure.ac | 50 ++-
> e2fsck/jfs_user.h | 62 ++-
> include/mingw/arpa/inet.h | 5 +
> include/mingw/sys/sysmacros.h | 8 +-
> lib/blkid/Android.bp | 1 -
> lib/blkid/devno.c | 10 +
> lib/blkid/getsize.c | 2 +-
> lib/blkid/probe.c | 10 +-
> lib/blkid/probe.h | 43 ---
> lib/blkid/save.c | 8 +
> lib/config.h.in | 100 ++++-
> lib/e2p/Android.bp | 4 -
> lib/e2p/fgetversion.c | 2 -
> lib/e2p/fsetversion.c | 1 -
> lib/e2p/getflags.c | 3 +-
> lib/e2p/getversion.c | 1 -
> lib/e2p/setversion.c | 1 -
> lib/et/Android.bp | 3 -
> lib/et/error_message.c | 10 +-
> lib/ext2fs/Android.bp | 2 -
> lib/ext2fs/bitops.c | 14 +-
> lib/ext2fs/bitops.h | 97 -----
> lib/ext2fs/ext2_io.h | 2 +
> lib/ext2fs/ext_attr.c | 2 -
> lib/ext2fs/gen_bitmap64.c | 6 +-
> lib/ext2fs/getsectsize.c | 12 +-
> lib/ext2fs/inline_data.c | 2 +-
> lib/ext2fs/jfs_compat.h | 4 -
> lib/ext2fs/mmp.c | 2 +-
> lib/ext2fs/windows_io.c | 12 +-
> lib/ss/Makefile.in | 5 +-
> lib/ss/execute_cmd.c | 2 -
> lib/ss/help.c | 2 -
> lib/ss/pager.c | 2 -
> lib/support/devname.c | 1 -
> lib/support/plausible.c | 7 +-
> lib/support/print_fs_flags.c | 60 +--
> lib/uuid/gen_uuid.c | 21 --
> misc/Android.bp | 3 -
> misc/create_inode.c | 36 +-
> misc/e4defrag.c | 30 +-
> misc/fuse2fs.c | 5 +-
> misc/mk_hugefiles.c | 134 +------
> misc/mke2fs.c | 22 +-
> misc/tune2fs.c | 7 +-
> misc/util.c | 5 +
> resize/resize2fs.c | 4 -
> util/android_config.h | 1 -
> util/subst.c | 4 +-
> 53 files changed, 1102 insertions(+), 812 deletions(-)
> create mode 100644 .github/workflows/ci.yml
> create mode 100644 include/mingw/arpa/inet.h
>
>
> base-commit: aad34909b6648579f42dade5af5b46821aa4d845
> --
> 2.39.0
>

Cheers, Andreas

Attachments:

signature.asc (873.00 B)
Message signed with OpenPGP