Skip to content

Conversation

@fabiobaltieri
Copy link
Member

@fabiobaltieri fabiobaltieri commented Sep 2, 2025

Hi, bumped into a build regression since #94779, one of our downstream application uses an interesting combination of configs that triggered a circular dependency build failure (which was a lot of fun to troubleshoot).

Managed to reproduce the issue upstream with

diff --git a/samples/basic/blinky/src/main.c b/samples/basic/blinky/src/main.c
index 4cab4969d94..5ebcbcf8a97 100644
--- a/samples/basic/blinky/src/main.c
+++ b/samples/basic/blinky/src/main.c
@@ -4,6 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#define _GNU_SOURCE
+#include <sys/types.h>
+
 #include <stdio.h>
 #include <zephyr/kernel.h>
 #include <zephyr/drivers/gpio.h>

and west build -p -b nrf52dk/nrf52832 samples/basic/blinky -DCONFIG_POSIX_API=y -DCONFIG_NEWLIB_LIBC=y

May look into adding a test case for this but for now I think the right thing to do is to split these on their own header.

--

This moves the declaration of the utf8 utils defined in lib/utils/utf8.c in their own header. Main reason to do this is that the current setup requried adding an include for sys/types.h in util.h, which can result in a build falure due to a circular header depdenecy when using:

CONFIG_POSIX_API=y
CONFIG_NEWLIB_LIBC=y
_GNU_SOURCE

the loop and error are:

  • include/sys/types.h:50: <- this is a NEWLIB one
  • include/zephyr/posix/sys/select.h:9:
  • include/zephyr/posix/posix_types.h:30:
  • include/zephyr/kernel.h:17:
  • include/zephyr/kernel_includes.h:25:
  • include/zephyr/sys/atomic.h:18:

include/zephyr/sys/util.h:705:1: error: unknown type name 'ssize_t'

@fabiobaltieri fabiobaltieri force-pushed the utf-util-header branch 3 times, most recently from 7ef92fc to a3f0834 Compare September 2, 2025 19:51
@fabiobaltieri fabiobaltieri marked this pull request as ready for review September 2, 2025 20:50
@fabiobaltieri
Copy link
Member Author

cc @iliar

@fabiobaltieri
Copy link
Member Author

Swapped to int, kept the separate header.

This moves the declaration of the utf8 utils defined in lib/utils/utf8.c
in their own header. Main reason to do this is that the current setup
requried adding an include for sys/types.h in util.h, which can result
in a build falure due to a circular header depdenecy when using:

CONFIG_POSIX_API=y
CONFIG_NEWLIB_LIBC=y
_GNU_SOURCE

the loop and error are:

- include/sys/types.h:50: <- this is a NEWLIB one
- include/zephyr/posix/sys/select.h:9:
- include/zephyr/posix/posix_types.h:30:
- include/zephyr/kernel.h:17:
- include/zephyr/kernel_includes.h:25:
- include/zephyr/sys/atomic.h:18:

include/zephyr/sys/util.h:705:1:
error: unknown type name 'ssize_t'

Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
pdgendt
pdgendt previously approved these changes Sep 4, 2025
Thalley
Thalley previously approved these changes Sep 4, 2025
Change utf8_count_chars return type to int and drop thesys/types.h, this
way the function does not depend on posix types.

Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
@fabiobaltieri
Copy link
Member Author

Sorry one more tweak: converted the local variable in utf8.c as well and drop the header from there too. Nothing is ever easy.

@fabiobaltieri
Copy link
Member Author

You know what, I may as well cherry pick the test while I'm at it.

This combination (with newlib) seems to catch some build failures due to
include depdenency, adding a test file so it gets caught in CI.

Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
@zephyrbot zephyrbot added the area: POSIX POSIX API Library label Sep 4, 2025
@fabiobaltieri
Copy link
Member Author

Ok so this is now a:

  • split the header on their own file
  • stop using types.h
  • add a test for the nasty use case that led to this

@cfriedt happy with me adding that file in the posix tests?

@nashif this is going to fail compliance because _GNU_SOURCE so it'll need org superpower to merge

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 4, 2025

@cfriedt
Copy link
Member

cfriedt commented Sep 4, 2025

LGTM - in general, we should avoid exposing "gnuisms" that become visible with _GNU_SOURCE.

For test purposes, I think it's OK to do as evidence to show that we don't break in the presence of that macro.

@fabiobaltieri
Copy link
Member Author

LGTM - in general, we should avoid exposing "gnuisms" that become visible with _GNU_SOURCE.

For test purposes, I think it's OK to do as evidence to show that we don't break in the presence of that macro.

Yeah of course, that's just for testing purposes, downstream users may do it, and we already have other tests that do it, and surprisingly some drivers code too.

@fabiobaltieri
Copy link
Member Author

@nashif? (note that there's a CI false positive)

@nashif nashif merged commit 9a27629 into zephyrproject-rtos:main Sep 8, 2025
31 of 32 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Bluetooth LE Audio Sep 8, 2025
@fabiobaltieri fabiobaltieri deleted the utf-util-header branch September 8, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants