Re: [PATCH 03/23] sigaction.2: Apply minor tweaks to Peter's patch

["Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>]

Hi Alex,

On 8/8/21 10:41 AM, Alejandro Colomar wrote:
> - Move example program to a new EXAMPLES section

Not a big thing, but perhaps it would have been nicer to have 
two patches, the first doing the pieces below, and the second
doing the step above. That would enable me to more easily see
what changes you made below. But, I am not sure; maybe that
is more work than is justified...

> - Invert logic in the handler to have the failure in the conditional path,
>   and the success out of any conditionals.
> - Use NULL, EXIT_SUCCESS, and EXIT_FAILURE instead of magic numbers
> - Separate declarations from code
> - Put function return type on its own line
> - Put function opening brace on its line
> 
> Cc: Peter Collingbourne <pcc@google.com>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
> ---
>  man2/sigaction.2 | 76 +++++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 33 deletions(-)
> 
> diff --git a/man2/sigaction.2 b/man2/sigaction.2
> index 4bf6f095e..18404dde1 100644
> --- a/man2/sigaction.2
> +++ b/man2/sigaction.2
> @@ -936,39 +936,6 @@ because they were introduced before Linux 5.11.
>  However, in general, programs may assume that these flags are supported,
>  since they have all been supported since Linux 2.6,
>  which was released in the year 2003.
> -.PP
> -The following example program exits with status 0 if
> -.B SA_EXPOSE_TAGBITS
> -is determined to be supported, and 1 otherwise.
> -.PP
> -.EX
> -#include <signal.h>
> -#include <stdio.h>
> -#include <unistd.h>
> -
> -void handler(int signo, siginfo_t *info, void *context) {
> -    struct sigaction oldact;
> -    if (sigaction(SIGSEGV, 0, &oldact) == 0 &&
> -        !(oldact.sa_flags & SA_UNSUPPORTED) &&
> -        (oldact.sa_flags & SA_EXPOSE_TAGBITS)) {
> -        _exit(0);
> -    } else {
> -        _exit(1);
> -    }
> -}
> -
> -int main(void) {
> -    struct sigaction act = {0};
> -    act.sa_flags = SA_SIGINFO | SA_UNSUPPORTED | SA_EXPOSE_TAGBITS;
> -    act.sa_sigaction = handler;
> -    if (sigaction(SIGSEGV, &act, 0) != 0) {
> -        perror("sigaction");
> -        return 1;
> -    }
> -
> -    raise(SIGSEGV);
> -}
> -.EE
>  .SH RETURN VALUE
>  .BR sigaction ()
>  returns 0 on success; on error, \-1 is returned, and
> @@ -1174,6 +1141,49 @@ This bug was fixed in kernel 2.6.14.
>  .SH EXAMPLES
>  See
>  .BR mprotect (2).
> +.PP
> +The following example program exits with status
> +.B EXIT_SUCCESS
> +if
> +.B SA_EXPOSE_TAGBITS
> +is determined to be supported, and
> +.B EXIT_FAILURE
> +otherwise.
> +.PP
> +.EX
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +void
> +handler(int signo, siginfo_t *info, void *context)
> +{
> +    struct sigaction oldact;
> +
> +    if (sigaction(SIGSEGV, NULL, &oldact) != 0 ||
> +        (oldact.sa_flags & SA_UNSUPPORTED) ||
> +        !(oldact.sa_flags & SA_EXPOSE_TAGBITS)) {
> +        _exit(EXIT_FAILURE);
> +    }
> +    _exit(EXIT_SUCCESS);
> +}
> +
> +int
> +main(void)
> +{
> +    struct sigaction act = {0};
> +
> +    act.sa_flags = SA_SIGINFO | SA_UNSUPPORTED | SA_EXPOSE_TAGBITS;
> +    act.sa_sigaction = &handler;
> +    if (sigaction(SIGSEGV, &act, NULL) != 0) {
> +        perror("sigaction");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    raise(SIGSEGV);
> +}
> +.EE
>  .SH SEE ALSO
>  .BR kill (1),
>  .BR kill (2),

All looks good. I also had some additional fixes (2 patches). See below.

Cheers,

Michael

diff --git a/man2/sigaction.2 b/man2/sigaction.2
index 5262b81c9..3225dc990 100644
--- a/man2/sigaction.2
+++ b/man2/sigaction.2
@@ -1161,9 +1161,9 @@ handler(int signo, siginfo_t *info, void *context)
 {
     struct sigaction oldact;
 
-    if (sigaction(SIGSEGV, NULL, &oldact) != 0 ||
-        (oldact.sa_flags & SA_UNSUPPORTED) ||
-        !(oldact.sa_flags & SA_EXPOSE_TAGBITS)) {
+    if (sigaction(SIGSEGV, NULL, &oldact) == \-1 ||
+            (oldact.sa_flags & SA_UNSUPPORTED) ||
+            !(oldact.sa_flags & SA_EXPOSE_TAGBITS)) {
         _exit(EXIT_FAILURE);
     }
     _exit(EXIT_SUCCESS);
@@ -1172,11 +1172,11 @@ handler(int signo, siginfo_t *info, void *context)
 int
 main(void)
 {
-    struct sigaction act = {0};
+    struct sigaction act = { 0 };
 
     act.sa_flags = SA_SIGINFO | SA_UNSUPPORTED | SA_EXPOSE_TAGBITS;
     act.sa_sigaction = &handler;
-    if (sigaction(SIGSEGV, &act, NULL) != 0) {
+    if (sigaction(SIGSEGV, &act, NULL) == \-1) {
         perror("sigaction");
         exit(EXIT_FAILURE);
     }


diff --git a/man2/sigaction.2 b/man2/sigaction.2
index 18404dde1..5262b81c9 100644
--- a/man2/sigaction.2
+++ b/man2/sigaction.2
@@ -266,13 +266,13 @@ This flag is meaningful only when establishing a signal handler.
 Used to dynamically probe for flag bit support.
 .IP
 If an attempt to register a handler succeeds with this flag set in
-.I act->sa_flags
+.I act\->sa_flags
 alongside other flags that are potentially unsupported by the kernel,
 and an immediately subsequent
 .BR sigaction ()
-call specifying the same signal number n and with non-NULL
+call specifying the same signal number and with a non-NULL
 .I oldact
-yields
+argument yields
 .B SA_UNSUPPORTED
 .I clear
 in
@@ -888,16 +888,16 @@ filter rule.
 The
 .BR sigaction ()
 call on Linux accepts unknown bits set in
-.I act->sa_flags
+.I act\->sa_flags
 without error.
 The behavior of the kernel starting with Linux 5.11 is that a second
 .BR sigaction ()
 will clear unknown bits from
-.IR oldact->sa_flags .
+.IR oldact\->sa_flags .
 However, historically, a second
 .BR sigaction ()
 call would typically leave those bits set in
-.IR oldact->sa_flags .
+.IR oldact\->sa_flags .
 .PP
 This means that support for new flags cannot be detected
 simply by testing for a flag in
@@ -919,7 +919,7 @@ in the signal handler itself.
 In kernels that do not support a specific flag,
 the kernel's behavior is as if the flag was not set,
 even if the flag was set in
-.IR act->sa_flags .
+.IR act\->sa_flags .
 .PP
 The flags
 .BR SA_NOCLDSTOP ,



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/