public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 1/3] libsupport: fix portability issues with the bthread.c
Date: Thu, 2 Apr 2026 21:16:24 -0700	[thread overview]
Message-ID: <20260403041624.GD6254@frogsfrogsfrogs> (raw)
In-Reply-To: <20260403040328.2385083-2-tytso@mit.edu>

On Fri, Apr 03, 2026 at 12:03:26AM -0400, Theodore Ts'o wrote:
> The function pthread_setname_np() is non-portable; that's what the
> "np" means.  In particular, on Mac systems, the function takes only a
> single argument, while on most other systems which have the function,
> it takes two arguments.
> 
> Also fix a problem where a 1-bit signed integer can only accept values
> of 0 or -1.  Change it to be a 1-bit unsigned integer, which can
> accept values of 0 or 1.  Clang will issue a warning if 1-bit signed
> integer are used incorrectly, and fail with -Werror.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  configure             |  6 ++++++
>  configure.ac          |  1 +
>  lib/config.h.in       | 24 +++++++++++++++---------
>  lib/support/bthread.c | 12 ++++++++++--
>  4 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/configure b/configure
> index 4da5439fe..b9a82dcec 100755
> --- a/configure
> +++ b/configure
> @@ -13887,6 +13887,12 @@ if test "x$ac_cv_func_pwrite64" = xyes
>  then :
>    printf "%s\n" "#define HAVE_PWRITE64 1" >>confdefs.h
>  
> +fi
> +ac_fn_c_check_func "$LINENO" "pthread_setname_np" "ac_cv_func_pthread_setname_np"
> +if test "x$ac_cv_func_pthread_setname_np" = xyes
> +then :
> +  printf "%s\n" "#define HAVE_PTHREAD_SETNAME_NP 1" >>confdefs.h
> +
>  fi
>  ac_fn_c_check_func "$LINENO" "qsort_r" "ac_cv_func_qsort_r"
>  if test "x$ac_cv_func_qsort_r" = xyes
> diff --git a/configure.ac b/configure.ac
> index ecef9df39..2473879fd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1269,6 +1269,7 @@ AC_CHECK_FUNCS(m4_flatten([
>  	pwrite
>  	pread64
>  	pwrite64
> +	pthread_setname_np
>  	qsort_r
>  	secure_getenv
>  	setmntent
> diff --git a/lib/config.h.in b/lib/config.h.in
> index 8ea7ec2b1..c6cbced5f 100644
> --- a/lib/config.h.in
> +++ b/lib/config.h.in
> @@ -46,9 +46,6 @@
>  /* Define to the version of FUSE to use */
>  #undef FUSE_USE_VERSION
>  
> -/* Define to 1 if fuse supports cache_readdir */
> -#undef HAVE_FUSE_CACHE_READDIR

Huh, there's a lot of churn in this file.  Do you have a magic script
somewhere that regenerates config.h.in?

I don't see any problems with this file's changes, but I could also not
scatter junk everywhere :)

> -
>  /* Define to 1 if you have the 'add_key' function. */
>  #undef HAVE_ADD_KEY
>  
> @@ -73,9 +70,6 @@
>  /* Define to 1 if you have the BSD-style 'qsort_r' function. */
>  #undef HAVE_BSD_QSORT_R
>  
> -/* Define to 1 if PR_SET_IO_FLUSHER is present */
> -#undef HAVE_PR_SET_IO_FLUSHER
> -
>  /* Define to 1 if you have the Mac OS X function
>     CFLocaleCopyPreferredLanguages in the CoreFoundation framework. */
>  #undef HAVE_CFLOCALECOPYPREFERREDLANGUAGES
> @@ -87,6 +81,9 @@
>  /* Define to 1 if you have the 'chflags' function. */
>  #undef HAVE_CHFLAGS
>  
> +/* Define to 1 if CLOCK_MONOTONIC is present */
> +#undef HAVE_CLOCK_MONOTONIC
> +
>  /* Define if the GNU dcgettext() function is already present or preinstalled.
>     */
>  #undef HAVE_DCGETTEXT
> @@ -136,9 +133,15 @@
>  /* Define to 1 if you have the 'fsync' function. */
>  #undef HAVE_FSYNC
>  
> +/* Define to 1 if FS_IOC_READ_VERITY_METADATA ioctl is available */
> +#undef HAVE_FS_IOC_READ_VERITY_METADATA
> +
>  /* Define to 1 if you have the 'ftruncate64' function. */
>  #undef HAVE_FTRUNCATE64
>  
> +/* Define to 1 if fuse supports cache_readdir */
> +#undef HAVE_FUSE_CACHE_READDIR
> +
>  /* Define to 1 if you have the <fuse.h> header file. */
>  #undef HAVE_FUSE_H
>  
> @@ -313,6 +316,9 @@
>  /* Define to 1 if you have the 'pread64' function. */
>  #undef HAVE_PREAD64
>  
> +/* Define to 1 if PR_SET_IO_FLUSHER is present */
> +#undef HAVE_PR_SET_IO_FLUSHER
> +
>  /* Define if you have POSIX threads libraries and header files. */
>  #undef HAVE_PTHREAD
>  
> @@ -322,6 +328,9 @@
>  /* Have PTHREAD_PRIO_INHERIT. */
>  #undef HAVE_PTHREAD_PRIO_INHERIT
>  
> +/* Define to 1 if you have the 'pthread_setname_np' function. */
> +#undef HAVE_PTHREAD_SETNAME_NP
> +
>  /* Define to 1 if you have the 'pwrite' function. */
>  #undef HAVE_PWRITE
>  
> @@ -699,7 +708,4 @@
>  /* Define to 1 on platforms where this makes time_t a 64-bit type. */
>  #undef __MINGW_USE_VC2005_COMPAT
>  
> -/* Define to 1 if CLOCK_MONOTONIC is present */
> -#undef HAVE_CLOCK_MONOTONIC
> -
>  #include <dirpaths.h>
> diff --git a/lib/support/bthread.c b/lib/support/bthread.c
> index 936ca0f0f..87eeb1b3d 100644
> --- a/lib/support/bthread.c
> +++ b/lib/support/bthread.c
> @@ -9,6 +9,7 @@
>   * %End-Header%
>   */
>  #include "config.h"
> +#ifdef HAVE_PTHREAD

Hmmm if we don't have pthreads, then shouldn't bthread.h also exclude
all the function declarations ifndef HAVE_PTHREAD?

>  #include <stdlib.h>
>  #include <errno.h>
>  #include <pthread.h>
> @@ -33,7 +34,7 @@ struct bthread {
>  	bthread_fn_t fn;
>  	void *data;
>  	unsigned int period; /* seconds */
> -	int can_join:1;
> +	unsigned int can_join:1;

That works, though I suppose you could change it to bool.

--D

>  };
>  
>  /* Wait for a signal or for the periodic interval */
> @@ -101,7 +102,13 @@ int bthread_create(const char *name,  bthread_fn_t fn, void *data,
>  	if (error)
>  		goto out_cond;
>  
> +#ifdef HAVE_PTHREAD_SETNAME_NP
> +#ifdef __APPLE__
> +	pthread_setname_np(name);
> +#else
>  	pthread_setname_np(bt->thread, name);
> +#endif
> +#endif
>  
>  	*btp = bt;
>  	return 0;
> @@ -178,7 +185,7 @@ int bthread_cancel(struct bthread *bt)
>  /* Ask the thread to cancel itself and wait for it */
>  void bthread_stop(struct bthread *bt)
>  {
> -	int need_join = 0;
> +	unsigned int need_join = 0;
>  
>  	pthread_mutex_lock(&bt->lock);
>  	switch (bt->state) {
> @@ -199,3 +206,4 @@ void bthread_stop(struct bthread *bt)
>  	if (need_join)
>  		pthread_join(bt->thread, NULL);
>  }
> +#endif /* HAVE_PTHREAD */
> -- 
> 2.51.0
> 

  reply	other threads:[~2026-04-03  4:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-03  4:03 [PATCH -e2fsprogs 0/3] Fix portability issues on MacOS Theodore Ts'o
2026-04-03  4:03 ` [PATCH 1/3] libsupport: fix portability issues with the bthread.c Theodore Ts'o
2026-04-03  4:16   ` Darrick J. Wong [this message]
2026-04-03 11:53     ` Theodore Tso
2026-04-03 15:15       ` Darrick J. Wong
2026-04-03 21:00         ` Theodore Tso
2026-04-03  4:03 ` [PATCH 2/3] libsupport: add a portable get_thread_id() function Theodore Ts'o
2026-04-03  4:17   ` Darrick J. Wong
2026-04-03  4:03 ` [PATCH 3/3] fuse2fs: fix build failure on systems which don't define EUCLEAN Theodore Ts'o
2026-04-03  4:18   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260403041624.GD6254@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox