| Message ID | dc7a7959808593c941fbf73f9f426f92632c0029.1527814874.git.keno@juliacomputing.com |
|---|---|
| State | New |
| Headers | show |
| Series | 9p: Add support for Darwin | expand |
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];]])], [gl_cv_promoted_mode_t='int'], [gl_cv_promoted_mode_t='mode_t']) ]) 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.
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 6222891..1f0b1b0 100644 --- a/hw/9pfs/9p-local.c +++ 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) { mode = credp->fc_mode; } if (credp->fc_rdev != -1) { @@ -416,7 +416,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp) return err; } } - if (credp->fc_mode != -1) { + if (credp->fc_mode != (mode_t)-1) { uint32_t tmp_mode = cpu_to_le32(credp->fc_mode); err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.mode", &tmp_mode, sizeof(mode_t), 0);
Comparisons of mode_t with -1 require an explicit cast, since mode_t is unsigned on Darwin. Signed-off-by: Keno Fischer <keno@juliacomputing.com> --- Changes from v1: Split from strchrnul change hw/9pfs/9p-local.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)