Email List: Xaustin-review-lX
[All Lists]

Re: Defect in XSH pthread_sigmask

To: yyyyyyyyyyyyyyy@xxxxxxxxxxxxx
Subject: Re: Defect in XSH pthread_sigmask
From: Loic Domaigne <yyyyyyyy@xxxxxxx>
Date: Tue, 15 Jul 2003 15:16:48 +0200 (MEST)
References: <200307111708.KAA32180@xxxxxx>
Hi! 

First, many thanks to Scott for his pertinent review. 


> I generally prefer to see 'strerror()' used with [fs]printf
> rather than perror().   perror cannot be used with most of the
> pthread-related interfaces because they don't set errno.

Yes, my mistake actually :-( Somehow, I was convinced that the pthreads APIs
returns 0 
on success, -1 on error and in this latter case the errno is set
accordingly. I checked quickly 
the standard, but didn't notice my mistake. Althought it is clearly stated
that (most of) the 
pthreads APIs returns an error code... Bad, bad, bad Loic :-)

Most of the defects that Scott reported result from this error.


> (NOTE TO SELF:  Submit ardvaark on perror() to add application usage
> regarding use of perror() with threaded applications (i.e. don't do it!)).

Yes, good idea! A question: does this comment only applied the Pthreads? 


>   The errno value should be a parameter to DIE and strerror()
> should be used to format the string for it.

Not the value of "errno", but more generally the error "returned" by the
APIs 
(i.e. errno if this variable is set, or as for the pthread_xxx functions,
the returned code).


>   __func__ is non standard C.

You mean, not "ANSI C".  As already mentionned by Ulrich, this is conform to
ISO C99.

Well, strictly speaking, __FILE__ would do a good job here. And I did make
this substitution
to test this code on platform without an ISO C99 compiler. 

I have introduced __func__ intentionally, to underline the fact that this
SUS  is based on 
ISO C99. 


> More generally, even the traditional functions define '-1' as
> the error return, not a negative number, so "if (rc < 0)" is almost
> never correct.   if (rc == -1) should be used instead to detect errors.

I hesitated quite a long between the two. I chose (rc < 0), thinking this
wouldn't make any 
difference: '-1' is a particular negative number.  I feel confortable, since
even famous 
authors on Unix programming test that way...  


> (ran in to this years ago with lseek() against /proc/pid/mem  on SVR4
> where a negative return value is legal when addresses have the MSB set).
> (the best check for lseek, with SEEK_SET, is to check that the return
> value matches the supplied offset value).

In light of this explanation, I do agree. I shall replace the (rc < 0)  with
(rc == -1)

 
> I'd prefer to see compound statements used (e.g. {}) with conditional
> statements (if, do, while) even if there is only one statement in the
> block.

That's a matter of taste. I usually also use compound statements, even if
there is only one
statement. Nevertheless, I thought the context here make it clear enough.

Anyway I shall make the change. 


I send you ASAP the revised version of this example.

Loic. 

-- 
+++ GMX - Mail, Messaging & more  http://www.gmx.net +++

Jetzt ein- oder umsteigen und USB-Speicheruhr als Prämie sichern!

<Prev in Thread] Current Thread [Next in Thread>