Re: Weekly reports - B

[Solar Designer <solar@openwall.com>]

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. ;-)

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?

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().  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.

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.

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).

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

Overall, I think the code could be made prettier.  I'll post an idea
(which you don't have to use, but you may) in another message.

Luka - when I say that something could be written better, etc., please
don't be discouraged.  On the contrary, this indicates that you're doing
something interesting, comment-worthy. :-)  I am merely trying to
provide constructive criticism and help the project in this way.

Thanks again,

Alexander