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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
URL:
<http://mail.opensolaris.org/pipermail/on-discuss/attachments/20090508/2518ff7c/attachment.bin>
