On Fri, May 08, 2009 at 07:22:33PM +0200, Roland Mainz wrote: > John Beck wrote: > > > > http://cr.opensolaris.org/~jbeck/mailwrapper/ > > > > Ceri did 99% of the work; I'm just sponsoring it. It's a simple port > > of mailwrapper from NetBSD, with some packaging and Makefile goo to > > make it work in ON. I'm targeting build 116 which opens next week, > > assuming I can get license review and approval soon enough, so let's > > say end of the day May 15 for feedback please. Thanks... > > >From a 5min look at > http://cr.opensolaris.org/~jbeck/mailwrapper/mailwrapper.patch it looks > Ok for me...
Thanks.
> ... some nits:
> * a -xstrconst in the CFLAGS would be nice (footprint reduction).
>
> * IMO it would be nice to compile this application with C99/XPG6 flags
> (footprint reduction/performance)
I don't really understand the implications of either of these two
suggestions, so I'm happy to do whatever common practice is.
> * While /usr/include/iso/stdio_iso.h defines |BUFSIZ| as |#define BUFSIZ
> 1024| I would still prefer a larger buffer to make sure it can hold a
> full path and additionally text. AFAIK a value of 2*PATH_MAX+1 may be
> nice.
OK with me.
> * usr/src/cmd/mailwrapper/mailwrapper.c
> - Line 57: int main(int, char *[], char *[]);
> Why is this prototype required ?
As John suggested, this was pulled from upstream and I didn't make any
changes that weren't required to make the application compile and work.
> - What about better messages for |malloc| failure handling, e.g. lines:
> 68 err(EX_TEMPFAIL, "malloc");
> 84 err(EX_TEMPFAIL, "strdup");
> etc.
> AFAIK at least the function name in these message _may_ be helpfull.
I agree with what you're saying regarding these messages but this is an
upstream issue too.
Of course, I am still (at least on paper) a committer to the FreeBSD
Project so it's possible that I can fix these in the upstream code.
That will require me to justify making changes in code that is also shared
by NetBSD (and OpenBSD maybe, I don't actually know) so might take a
while and therefore I'd prefer to not make the changes for OpenSolaris
without making them upstream first (or at the same time at least).
> - It may be nice to use |strtok_r()| instead of |strtok()| (performance
> (|strtok()| uses |tsdalloc()| and then calls |strtok_r()|))
The strtok() calls are mine (FreeBSD uses its strsep() in the original
code) so I'm happy to make that replacement.
Cheers,
Ceri
--
That must be wonderful! I don't understand it at all.
-- Moliere
pgpjmrcbmwdcb.pgp
Description: PGP signature
_______________________________________________ opensolaris-code mailing list opensolaris-code@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
