On Fri, Jun 12, 2009 at 11:37:01AM +0200, Darren Reed wrote: > The webrev below contains the changes required to fix CR#6271923 > according to PSARC/2009/332. If someone who is faimilar with > setproject.c could comment on those changes, that would be wonderful. > > http://cr.opensolaris.org/~darrenr/6271923-20090611/
- $SRC/pkgdefs/SUNWcsr/prototype_com:231 Er, you need to change the class of the etc/project entry from 'preserve' to 'project'. You also need to update $SRC/pkgdefs/SUNWcsr/pkginfo.tmpl's CLASSES to add the project class. (Have you tested this? The best way to test SVR4 CAS, IIRC, is to do a netinstall using an S8 boot root, since that is the most restrictive context in which CAS can run. Yes, CAS testing is a major PITA.) - $SRC/lib/libproject/common/setproject.c:93 Typo: s/behvaviour/behavior/ (or behaviour, if you want alternate spelling). - $SRC/lib/libproject/common/setproject.c:93-97,111-116 The comment is hard to understand. I think you're trying to say that setproject(3PROJECT) is not allowed to fail because of failure to clear an rctl. I agree that that's the old behavior and that the manpage's description of error conditions makes doing anything other than looping in this case... seem wrong. But looping forever seems wrong to me too, particularly given that setproject() is not atomic (i.e., it can fail after having put the process in a new task and will leave the process in the new task). Any chance that we can get rid of this potentially infinite loop? At least file a CR, if there isn't one already. Anyways, how about changing the comment at 93-97 like so: * In case (2) we may not fail, therefore we loop, possibly forever. - $SRC/lib/libproject/common/setproject.c:118 I don't see the point of 'tried' and 'ok' and this check. If we broke out of the while loop it's because the rctl was cleared, and whether or not that was because pr_setrctl() succeeded seems to me to be irrelevant. Also, 'tried' could wraparound, so if this check were important then it's buggy; switching to 64-bit integers would make this exceedingly unlikely :) but, why bother with the check at all (see above)? - $SRC/lib/libproject/common/setproject.c:242 Nit: elsewhere in this file the return value of str*cmp() is compared, rather than using !str*cmp() as you do. Please keep the style consistent. Done. Nico --
