On Thu, Nov 06, 2008 at 05:27:12PM -0800, [EMAIL PROTECTED] wrote:
> http://cr.opensolaris.org/~johansen/webrev-4612/
Looks awesome, thanks. Just a few small points here ...
pspawn.c:
- line 52: sometimes you use this, sometimes you use "rc != 0".
- line 105: Is a mode_t really an unsigned long? I'm not sure how to
read the #ifdefs in sys/types.h, so I can't tell. Should there be a
size test, or will the uint_t version never be used?
- line 192: Shouldn't this be "pkg.pspawn.FileAction"?
- line 210: I don't think this is right, as many of the things this
implies aren't true.
- line 264: if args_seq is NULL, you should return, or the TypeError
won't be thrown, and things could get screwy.
- line 288: if we get here, you won't Py_DECREF(env_seq).
- line 357, 407: I wonder if closefds / closeall_fds should be renamed
not to imply that they actually close the file descriptors, but to
imply that they set up file descriptors for closure at spawn time.
- line 386: do you ever actually use except_fd? It seems like you should
either remove it or allow it to be a sequence of fds.
- line 413: Why do you need this block? Our other modules seem to get
along fine without it, but maybe they're wrong.
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
