posix: signal: only define sigset_t if not defined

[JordanYates]

Only define sigset_t if it has not already been defined externally, for example in picolibc/include/sys/signal.h.

This appears to be standard way of preventing redefinitions, see:

• https://github.com/f32c/arduino/blob/7d8cbb33fab873e2d2c6660be4eb42e09ac3eee0/hardware/fpga/f32c/system/include/sys/select.h#L47
• https://github.com/bminor/newlib/blob/730703bdb8e1ae671517efaa0b9e273eef52d724/newlib/libc/include/sys/select.h#L18
• https://github.com/picolibc/picolibc/blob/58d6157cc2135df5043d62c3e89feedc20ffcd57/newlib/libc/include/sys/signal.h#L45

[cfriedt]

FWICT, the external definition in picolibc and newlib is broken for the posix use case if realtime signals are enabled, depending on the platform, as unsigned long (__sigset_t) is only 32 bits on 32-bit platforms and realtime signals can add several extra bits.

Curious what @ycsin thinks about it though.

Timing-wise, I was actually in the process of defining some signal facilities at the kernel level too, so it will be interesting to see how this plays out.

[ycsin]

Curious what @ycsin thinks about it though.

I wonder how it will work out since the underlying implementation should be different?

[JordanYates]

Curious what @ycsin thinks about it though.

I wonder how it will work out since the underlying implementation should be different?

This only fixs the first problem, which is the compiler erroring out because of a redefinition of the struct.
It makes no claims as to whether the underlying implementations will work properly.
I was hitting this case in external module code where both this file and standard library signal file were both included, even though they weren't being used AFAICT.

[cfriedt]

@JordanYates - this is still on my radar, but there is a pretty big change that I think it overlaps with. My hopes are that we can have a single location for defining sigset_t as a result.

[JordanYates]

@JordanYates - this is still on my radar, but there is a pretty big change that I think it overlaps with. My hopes are that we can have a single location for defining sigset_t as a result.

Happy for a more complete solution upstream instead of this bandaid.

[cfriedt]

A more complete solution was scheduled as a bugfix for v3.7.0, but I have my doubts that the prerequisite PRs will make it in, so it looks like the fix for this will also not be a part of v3.7

[MaureenHelm]

@cfriedt please revisit

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[kartben]

ping @cfriedt @ycsin @aescolar @keith-packard

[cfriedt]

I think if the definition for sigset were moved to a separate header, <zephyr/posix/sys/_sigset.h>, and that header was included here, I would be more inclined to approve.

[JordanYates]

I think if the definition for sigset were moved to a separate header, <zephyr/posix/sys/_sigset.h>, and that header was included here, I would be more included to approve.

Why is a dedicated header preferred? It's not the pattern I see in the other usage.

[cfriedt]

Why is a dedicated header preferred? It's not the pattern I see in the other usage.

Because that's the change that I have had staged in another WIP PR for a while.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[JordanYates]

Why is a dedicated header preferred? It's not the pattern I see in the other usage.

Because that's the change that I have had staged in another WIP PR for a while.

I don't think it is worth complicating this PR in order to align with a PR that has been WIP for almost a year, unless there is some reason why merging this change is incompatible with whatever you are working on. This is a trivial change that solves a real problem.

[JordanYates]

Damn, I missed the first birthday of this PR :)

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.