Mike Gerdts wrote: > On Sun, Mar 9, 2008 at 12:27 PM, Sarah Jelinek <Sarah.Jelinek at sun.com> > wrote: > >> http://cr.opensolaris.org/~sjelinek/bashrc_bug/ >> > > Since I reported this one, I decided to take a look at the fix... > > 1964 - 1965: snprintf format has 6 %s's but only 5 arguments after the format. > Hmm.... That's strange. I didn't notice that and this worked. Let me take a look. > 1964 - 1974: nit: chown and chgrp commands could be combined as > > (void) snprintf(cmd, sizeof (cmd), > "/usr/bin/chown %s:%s %s/%s/%s/%s", save_login_name, "staff", > target, home, save_login_name, bashrc); > > That being said... I don't have a good feel for how this code ties > into the overall environment, but it seems as though this is likely to > be running from a live cd and writing to an alternate root. In that > case, the user represented by save_login_name must exist in the passwd > name service on the cd and the alternate root environment. Given that > the home directory permissions are set via: > > 1782 uid = (uid_t)strtol(USER_UID, (char **)NULL, 10); > 1783 gid = (gid_t)strtol(USER_GID, (char **)NULL, 10); > 1784 if (uid != 0 && gid != 0) { > 1785 (void) chown(homedir, uid, gid); > > It would seem to be more efficient (who cares, for this) and less > error prone to use the same approach for the bashrc. > > At this point in time, the user does exist in the passwd file in the alternate root. Not sure why they would need to exist in the passwd file on the livecd for this? I certainly could do this calling chown(). I can also user the defined uid and gid values. If this ever changes in that these are no longer defines but values input by the users during install this will have to be modified.
Thanks for the review, sarah
