Skip to content

Commit

Permalink
posix: mqueue: pop mode as int with va_arg()
Browse files Browse the repository at this point in the history
There was some discussion about whether it was suitable to have
an architecture-specific workaround in mqueue.c after that
workaround was copied to a different source file in a PR.

The original issue was that newlib and picolibc declare mode_t
to be unsigned short instead of unsigned long when __svr4__
is not defined along with __sparc__. This is specifically
impactful, because va_arg() deals (mainly) with 32-bit and
64-bit values that are all naturally aligned to 4 bytes.

#if defined(__sparc__) && !defined(__sparc_v9__)
#ifdef __svr4__
typedef unsigned long __mode_t;
#else
typedef unsigned short __mode_t;
#endif

A uint16_t is naturally aligned to 2 bytes, so not only would
a 16-bit mode_t be corrupted, it would also generate a warning
with recent gcc versions which is promoted to error (rightfully
so) when run through CI.

mqueue.c:61:35: error: 'mode_t' {aka 'short unsigned int'} is
  promoted to 'int' when passed through '...' [-Werror]
   61 |                 mode = va_arg(va, mode_t);

Instead of using an architecture-specific workaround, simply
add a build assert that the size of mode_t is less than or
equal to the size of an int, and use an int to retrieve it
via va_arg().

Signed-off-by: Christopher Friedt <cfriedt@meta.com>

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date:      Fri Dec 29 10:06:44 2023 -0500
#
# On branch posix-mqueue-always-use-int-for-mode-t-va-arg
# Changes to be committed:
#	modified:   lib/posix/mqueue.c
#	modified:   tests/posix/common/testcase.yaml
#
  • Loading branch information
cfriedt committed Jan 1, 2024
1 parent e67b12e commit 10156f5
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 15 deletions.
14 changes: 2 additions & 12 deletions lib/posix/mqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,6 @@ static int receive_message(mqueue_desc *mqd, char *msg_ptr, size_t msg_len,
k_timeout_t timeout);
static void remove_mq(mqueue_object *msg_queue);

#if defined(__sparc__)
/*
* mode_t is defined as "unsigned short" on SPARC newlib. This type is promoted
* to "int" when passed through '...' so we should pass the promoted type to
* va_arg().
*/
#define PROMOTED_MODE_T int
#else
#define PROMOTED_MODE_T mode_t
#endif

/**
* @brief Open a message queue.
*
Expand All @@ -69,7 +58,8 @@ mqd_t mq_open(const char *name, int oflags, ...)

va_start(va, oflags);
if ((oflags & O_CREAT) != 0) {
mode = va_arg(va, PROMOTED_MODE_T);
BUILD_ASSERT(sizeof(mode_t) <= sizeof(int));
mode = va_arg(va, unsigned int);
attrs = va_arg(va, struct mq_attr*);
}
va_end(va);
Expand Down
3 changes: 0 additions & 3 deletions tests/posix/common/testcase.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
common:
filter: not (CONFIG_NATIVE_BUILD and CONFIG_EXTERNAL_LIBC)
# FIXME: qemu_leon3 is excluded because of the alignment-related failure
# reported in the GitHub issue zephyrproject-rtos/zephyr#48992.
platform_exclude:
- qemu_leon3
- native_posix
- native_posix_64
tags: posix
Expand Down

0 comments on commit 10156f5

Please sign in to comment.