[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 12/20] 9p: darwin: Explicitly cast comparison

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2 12/20] 9p: darwin: Explicitly cast comparisons of mode_t with -1
Date: Fri, 29 Jun 2018 15:32:23 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 05/31/2018 08:26 PM, Keno Fischer wrote:
Comparisons of mode_t with -1 require an explicit cast, since mode_t
is unsigned on Darwin.

It's not JUST that mode_t is unsigned (an unsigned int compares just fine against -1), but ALSO that mode_t has unspecified width. That is, you cannot portably assume whether mode_t is smaller, equivalent, or larger rank than int. If it is smaller, then you can't use mode_t in va_arg(), and mode_t will promote to signed int, whether or not mode_t is unsigned; but '((mode_t)-1) == -1' is going to be false if mode_t is unsigned (because the cast truncates the sign extension bits into a positive value). Conversely, since mode_t can be larger than int (although I know of no such platform that does so), blindly using 'int' when trying to parse a mode_t argument through va_arg() will truncate bits.

So, for example, to portably write a wrapper around open(), you HAVE to use hairy constructions like:

mode = (sizeof (mode_t) < sizeof (int)
        ? va_arg (ap, int)
        : va_arg (ap, mode_t));

or, to avoid spurious compiler warnings on the branch not taken, define a macro learned at configure time. This is what gnulib does, for example:

  AC_CACHE_CHECK([for promoted mode_t type], [gl_cv_promoted_mode_t], [
dnl Assume mode_t promotes to 'int' if and only if it is smaller than 'int', dnl and to itself otherwise. This assumption is not guaranteed by the ISO C
    dnl standard, but we don't know of any real-world counterexamples.
    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <sys/types.h>]],
      [[typedef int array[2 * (sizeof (mode_t) < sizeof (int)) - 1];]])],
  AC_DEFINE_UNQUOTED([PROMOTED_MODE_T], [$gl_cv_promoted_mode_t],
[Define to the type that is the result of default argument promotions of type mode_t.])

+++ b/hw/9pfs/9p-local.c
@@ -310,7 +310,7 @@ update_map_file:
      if (credp->fc_gid != -1) {
          gid = credp->fc_gid;
-    if (credp->fc_mode != -1) {
+    if (credp->fc_mode != (mode_t)-1) {

At any rate, this is the correct portability fix for this code.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]