valgrind-users

[Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Matthew E. <ma...@gs...>]

I was reviewing the source to corecheck/tests/sigkill.c the other day and
noticed some questionable code, namely:

a) setting errno = 0.  errno should never be set by user applications (as
per SUSv3)
b) calling perror() without checking the return code of a library function
first (as per SUSv3)

The following patch addresses these issues and preserves the existing
behaviour of the sigkill.c code.

Comments welcome.

--- corecheck/tests/sigkill.c.orig      Fri Jul  4 16:25:28 2003
+++ corecheck/tests/sigkill.c   Fri Jul  4 16:41:30 2003
@@ -17,18 +17,19 @@
   struct sigaction  sa;

   int i;
+  int rc;
   for (i = 1; i <= 65; i++) {
      sa.sa_flags   = 0;
      sigemptyset( &sa.sa_mask );
      sa.sa_handler = abend;
-     errno = 0;
      fprintf(stderr,"setting signal %d: ", i);
-     sigaction (i /*SIGKILL*/, &sa, NULL);
-     perror ("");
-     errno = 0;
+     rc = sigaction (i /*SIGKILL*/, &sa, NULL);
+     if (rc) perror ("");
+     else fprintf(stderr,"Success\n");
      fprintf(stderr,"getting signal %d: ", i);
-     sigaction (i /*SIGKILL*/, NULL, &sa);
-     perror ("");
+     rc = sigaction (i /*SIGKILL*/, NULL, &sa);
+     if (rc) perror ("");
+     else fprintf(stderr,"Success\n");
      fprintf(stderr,"\n");
   }
   return 0;

--
Matt Emmerton

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Dan K. <da...@ke...>]

Matthew Emmerton wrote:
> I was reviewing the source to corecheck/tests/sigkill.c the other day and
> noticed some questionable code, namely:
> 
> a) setting errno = 0.  errno should never be set by user applications (as
> per SUSv3)

You may have misread the standard.  I just checked
http://www.opengroup.org/onlinepubs/007904975/functions/errno.html
and it says that no Unix library or system call will clear errno;
it doesn't say the app can't.  In fact, it suggests it's ok for
the app to:

"An application that needs to examine the value of errno to determine the error should
set it to 0 before a function call, then inspect it before a subsequent function call."

- Dan

-- 
Dan Kegel
http://www.kegel.com
http://counter.li.org/cgi-bin/runscript/display-person.cgi?user=78045

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Matthew E. <ma...@co...>]

----- Original Message ----- 
From: "Dan Kegel" <da...@ke...>
To: "Matthew Emmerton" <ma...@gs...>
Cc: <val...@li...>
Sent: Friday, July 04, 2003 6:36 PM
Subject: Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c


> Matthew Emmerton wrote:
> > I was reviewing the source to corecheck/tests/sigkill.c the other day
and
> > noticed some questionable code, namely:
> >
> > a) setting errno = 0.  errno should never be set by user applications
(as
> > per SUSv3)
>
> You may have misread the standard.  I just checked
> http://www.opengroup.org/onlinepubs/007904975/functions/errno.html
> and it says that no Unix library or system call will clear errno;
> it doesn't say the app can't.  In fact, it suggests it's ok for
> the app to:
>
> "An application that needs to examine the value of errno to determine the
error should
> set it to 0 before a function call, then inspect it before a subsequent
function call."

However, sigaction() will return a negative error code should an error
occur.  Thus, setting errno = 0 prior to calling sigaction() is unneccessary
since the return code (not errno) is what that application should be
checking to determine if an error has occurred.

Small details like this are very important for some of the non-Linux porting
that is underway for valgrind.

--
Matt Emmerton

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Dan K. <da...@ke...>]

Matthew Emmerton wrote:
> However, sigaction() will return a negative error code should an error
> occur.  Thus, setting errno = 0 prior to calling sigaction() is unneccessary
> since the return code (not errno) is what that application should be
> checking to determine if an error has occurred.

If you're saying "we don't need to set errno to zero", that's fine.
All I'm pointing out is that your earlier statement, "SuSv3 says
it's illegal to set errno to zero", is false.

> Small details like this are very important for some of the non-Linux porting
> that is underway for valgrind.

Ah, are we porting valgrind to non-Posix-compliant systems
where it's illegal to set errno to zero?
- Dan

-- 
Dan Kegel
http://www.kegel.com
http://counter.li.org/cgi-bin/runscript/display-person.cgi?user=78045

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Matthew E. <ma...@co...>]

> Matthew Emmerton wrote:
> > However, sigaction() will return a negative error code should an error
> > occur.  Thus, setting errno = 0 prior to calling sigaction() is
unneccessary
> > since the return code (not errno) is what that application should be
> > checking to determine if an error has occurred.
>
> If you're saying "we don't need to set errno to zero", that's fine.
> All I'm pointing out is that your earlier statement, "SuSv3 says
> it's illegal to set errno to zero", is false.

Yes.  Touche.

> > Small details like this are very important for some of the non-Linux
porting
> > that is underway for valgrind.
>
> Ah, are we porting valgrind to non-Posix-compliant systems
> where it's illegal to set errno to zero?

No, we're porting to an almost-SUSv3 compliant system (FreeBSD) where
strerror/perror does not return "Success" for errno 0, since errno 0 is
technically not an error and the use of strerror/perror in that situation is
undefined.

--
Matt Emmerton

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Dan K. <da...@ke...>]

Matthew Emmerton wrote:
> No, we're porting to an almost-SUSv3 compliant system (FreeBSD) where
> strerror/perror does not return "Success" for errno 0, since errno 0 is
> technically not an error and the use of strerror/perror in that situation is
> undefined.

The patch looks good.  I have no objection to it, in fact, it's an improvement
regardless of which OS you run it on.  Apps should not set errno unless
they have a good reason, since at least one earlier version of some standard
didn't allow it... can't recall which...
- Dan

-- 
Dan Kegel
http://www.kegel.com
http://counter.li.org/cgi-bin/runscript/display-person.cgi?user=78045

Re: [Valgrind-users] Suggested changes to corecheck/tests/sigkill.c

[Dirk M. <dm...@gm...>]

On Fre, 04 Jul 2003, Matthew Emmerton wrote:

> The following patch addresses these issues and preserves the existing
> behaviour of the sigkill.c code.

Committed, thanks. 


-- 
Dirk