[vim/vim] Fix compiler warning about sigset() (PR #12313)

h_east

unread,

Apr 29, 2023, 12:11:18 PM4/29/23

to vim/vim, Subscribed

The compiler outputs 21 warnings about sigset() in os_unix.c.
Below is one of them.

os_unix.c: In function 'sig_winch':
os_unix.c:872:5: warning: 'sigset' is deprecated: Use the signal and sigprocmask functions instead [-Wdeprecated-declarations]
  872 |     signal(SIGWINCH, (void (*)(int))sig_winch);
      |     ^~~~~~
In file included from /usr/include/x86_64-linux-gnu/sys/param.h:28,
                 from os_unix.h:60,
                 from vim.h:244,
                 from os_unix.c:21:
/usr/include/signal.h:367:23: note: declared here
  367 | extern __sighandler_t sigset (int __sig, __sighandler_t __disp) __THROW
      |                       ^~~~~~

My env.

$ cat /etc/os-release | grep "VERSION="
VERSION="22.04.2 LTS (Jammy Jellyfish)"

$ gcc --version
gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0

$ ldd --version
ldd (Ubuntu GLIBC 2.35-0ubuntu3.1) 2.35

Cause code

[os_unixx.h: L16]

#if defined(HAVE_SIGSET) && !defined(signal)
# define signal sigset
#endif

signal is replaced with sigset here.

!defined(signal) is not right. This does not determine whether there is a signal() function or not.
sigset() is obsolete than signal().
Both are obsoleted, but my gcc outputs warnings only for sigset().

Ubuntu Manpage:
sigset(3)

This interface is made obsolete by sigsuspend(2) and sigaction(2).

signal(2)

WARNING: the behavior of signal() varies across UNIX versions, and has also varied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below.

Archlinux Manpage:
sigset(3)

These functions are provided in glibc as a compatibility interface for programs that make use of the historical System V signal API. This API is obsolete: new applications should use the POSIX signal API (sigaction(2), sigprocmask(2), etc.)

signal(2)

WARNING: the behavior of signal() varies across UNIX versions, and has also varied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below.

Solution

In this PR, I just stopped using sigset().
The next step is to replace signal() with sigaction(), I think. (Wrapping it with mch_signal()?)

--
Best regards,
Hirohito Higashi (h_east)

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/12313

Commit Summary

c9ec82a Fix compiler warning about sigset()

File Changes

(5 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Apr 29, 2023, 12:22:03 PM4/29/23

to vim/vim, Subscribed

Codecov Report

Merging #12313 (c9ec82a) into master (971cd2b) will decrease coverage by 3.89%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #12313      +/-   ##
==========================================
- Coverage   82.04%   78.16%   -3.89%     
==========================================
  Files         160      150      -10     
  Lines      193389   151372   -42017     
  Branches    43419    39047    -4372     
==========================================
- Hits       158675   118317   -40358     
+ Misses      21837    20816    -1021     
+ Partials    12877    12239     -638

Flag	Coverage Δ
huge-clang-none	`?`
linux	`?`
mingw-x64-HUGE	`76.57% <ø> (+<0.01%)`	⬆️
mingw-x86-HUGE	`77.03% <ø> (-0.01%)`	⬇️
windows	`78.16% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 150 files with indirect coverage changes

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

h_east

unread,

Apr 29, 2023, 12:53:05 PM4/29/23

to vim/vim, Push

@h-east pushed 2 commits.

37dc744 Revert "Fix compiler warning about sigset()"
fcb03d1 Fixed to not affect BSD and MACOS.

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Apr 29, 2023, 1:02:41 PM4/29/23

to vim/vim, Subscribed

> The compiler outputs 21 warnings about `sigset()` in os_unix.c.
> Below is one of them.
>

> ```bash

> os_unix.c: In function 'sig_winch':
> os_unix.c:872:5: warning: 'sigset' is deprecated: Use the signal and sigprocmask functions instead [-Wdeprecated-declarations]
> 872 | signal(SIGWINCH, (void (*)(int))sig_winch);
> | ^~~~~~
> In file included from /usr/include/x86_64-linux-gnu/sys/param.h:28,
> from os_unix.h:60,
> from vim.h:244,
> from os_unix.c:21:
> /usr/include/signal.h:367:23: note: declared here
> 367 | extern __sighandler_t sigset (int __sig, __sighandler_t __disp) __THROW
> | ^~~~~~

> ```
>
> ## My env.

> ```
> $ cat /etc/os-release | grep "VERSION="
> VERSION="22.04.2 LTS (Jammy Jellyfish)"
>
> $ gcc --version
> gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
>
> $ ldd --version
> ldd (Ubuntu GLIBC 2.35-0ubuntu3.1) 2.35

> ```
>
> ## Cause code
>
> [os_unixx.h: L16]
> ```c

> #if defined(HAVE_SIGSET) && !defined(signal)
> # define signal sigset
> #endif
> ```
>
> `signal` is replaced with `sigset` here.

> 1. `!defined(signal)` is not right. This does not determine whether

> there is a `signal()` function or not.

I think it was right. This is not to check whether a signal() function
exists (it nearly always does exist) but whether the macro already
exists. Whether signal() or sigset() works better heavily depends on
the system used.

> 2. `sigset()` is obsolete than `signal()`.
> Both are obsoleted, but my gcc outputs warnings only for `sigset()`.

This is only on Ubuntu, it probably also applies to other recent Linux
systems.

> Ubuntu Manpage:

Keep in mind that Linux manpages can only be trusted for Linux. Whether
they accurately describe other systems isn't guaranteed anywhere. There
simply are too many different Unix-like systems, each with their own
problems (aka bugs). Linux man page maintainers may or may not have
taken the time to dive into this.

> [sigset(3)](https://manpages.ubuntu.com/manpages/kinetic/man2/sighold.2freebsd.html)

>
> > This interface is made obsolete by sigsuspend(2) and sigaction(2).
>

> [signal(2)](https://manpages.ubuntu.com/manpages/kinetic/en/man2/signal.2.html)

>
> > WARNING: the behavior of signal() varies across UNIX versions, and has also varied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below.
>
> Archlinux Manpage:

> [sigset(3)](https://man.archlinux.org/man/sigset.3.en)

>
> > These functions are provided in glibc as a compatibility interface for programs that make use of the historical System V signal API. This API is obsolete: new applications should use the POSIX signal API (sigaction(2), sigprocmask(2), etc.)
>

> [signal(2)](https://man.archlinux.org/man/signal.2)

>
> > WARNING: the behavior of signal() varies across UNIX versions, and has also varied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below.

Although these warnings may be useful when writing new code, the Vim
implementation has existed for a very long time. If there would be an
actual problem with how signal() and related functions are used, I would
be more eager to make changes. But since I'm not aware of actual
problems, making changes has a risk of introducing problems instead of
avoiding them.

> ## Solution
>
> In this PR, I just stopped using `sigset()`.

I do not like the way this is done. Since the warnings are for Linux it
would be good to only make the changes for Linux.

> The next step is to replace `signal()` with `sigaction()`, I think.
> (Wrapping it with `mch_signal()`?)

Only when we know the sigaction() implementation on all relevant system
is actually working the way we use it. Considering the many mistakes
made in the past, it's not unlikely that new solutions also have
mistakes. I would not be surprised if the problems mentioned in those
warnings are taken care of, while things that were working fine before
are now broken. It most likely requires using a configure check that
the system has proper sigaction() support. I have no idea how to test
that though. Does this depend on a kernel version? glibc version?

--
Any resemblance between the above views and those of my employer, my terminal,
or the view out my window are purely coincidental. Any resemblance between
the above and my own views is non-deterministic. The question of the
existence of views in the absence of anyone to hold them is left as an
exercise for the reader. The question of the existence of the reader is left
as an exercise for the second god coefficient. (A discussion of
non-orthogonal, non-integral polytheism is beyond the scope of this article.)
(Ralph Jennings)

/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

h_east

unread,

Apr 29, 2023, 1:31:38 PM4/29/23

to vim/vim, Subscribed

@brammool
Thanks for the reply. I understand.
It seems that test_suspend is failing in some cases on Linux as well.
it's difficult... Closing.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

h_east

unread,

Apr 29, 2023, 1:31:40 PM4/29/23

to vim/vim, Subscribed

Closed #12313.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward