Re: Weekly reports - B

[Rich Felker <dalias@aerifal.cx>]

On Mon, Jun 13, 2011 at 06:11:30AM +0400, Solar Designer wrote:
> Luka, Rich -
> 
> On Thu, Jun 09, 2011 at 10:20:45PM +0200, Luka Mar??eti?? wrote:
> > Anyway, here's something... *hangs head*
> > https://github.com/paxcoder/cluts
> 
> Thanks for posting this.  I took a look.  This is good for the start,
> but we obviously need a lot more. ;-)

Yes. :)

> Sorry to remind you, but we need Luka's code placed under an Open Source
> license - and not only when cluts is "finished".  Each week's update
> must be properly licensed.  Can one or both of you please propose a
> license you're comfortable with?

Let's make it (new) BSD. Is that okay?

> Some assorted comments on the code, in arbitrary order:
> 
> For jumping out of a signal handler, you need to use sigjmp_buf,
> sigsetjmp(), and siglongjmp().

This only matters if you want the signal mask to be restored, which we
DO want, but another way to achieve the same thing would be to install
the signal handler with SA_NOMASK so the SIGSEGV never gets masked to
begin with (another SIGSEGV should not happen inside the signal
handler, and if it did while it was blocked, we'd be screwed anyway).

BTW another way to restore the signal mask, especially if you want it
to be restored to the mask at the time the signal was generated rather
than at the time the jump buffer was created, is to use the SA_SIGINFO
signal handler form and read the saved sigset_t from the ucontext_t
argument and restore it yourself with sigprocmask. :-)

> Even so, some failed libc functions
> might leave stdio (or something else) in an inconsistent state.  This is
> probably irrelevant to simple string functions testing, but it will be
> relevant to some other tests.  Thus, since we don't expect SIGSEGVs to
> be frequent, maybe it'd be better to switch to forking child processes
> (which must print something specific to fd 1 to indicate success)?
> Or we can use both approaches - in different cases, as appropriate.

In the case of testing string functions, the test framework setup a
very narrow class of "likely causes" for the SIGSEGV, and unless the
functions are hopelessly broken, we can assume any SIGSEGV was caused
by the condition that was being tested for. Therefore, in this case I
don't think we have to worry about corrupt state and such. Note that
POSIX does not require string functions to be async-signal-safe, for
some odd reason, but as far as I know all real-world implementations
including glibc guarantee that they are (I found a discussion of glibc
strstr optimization where use of malloc was rejected because it would
violate their requirement that they want it to be async-signal-safe).
Thus they should not have any internal state that could get corrupted.

> When you declare identifiers at the global scope in a file, but don't
> need them exported to other source files, please make them "static" to
> prevent inadvertent use from another source file.

Agreed.

> What do you mean by "#define _XOPEN_SOURCE 9001"?  I think the highest
> value currently defined is 700, and going too high may actually prevent
> this from working (e.g., on Solaris).

I noticed this too. Also you're defining it after including headers,
which has no effect but invoking UB. To use feature test macros they
must be defined before any system headers are included.

> Please avoid assignments to errno.  Use your own variable instead.

Is this just a stylistic preference, or do you have a reason it could
be problematic?

Rich