Ethan,
thank you very much for review !
Jan
On 06/21/10 05:57 PM, Ethan Quach wrote:
On 06/21/10 00:47, Jan Damborsky wrote:
On 06/18/10 08:38 PM, Ethan Quach wrote:
452 - I think its a bit dangerous to hardcode "rpool" as the
pool name here (even though currently in AI we always expect
to install into 'rpool') It would be nice if you just wrote a small
bit of code to figure out the name of the pool on which the
dirname of $home_mntpoint file system lives, and fill that in
as the poolname. That way it'll just work when we use this with
text-installer, and when AI supports pool naming and multiple
pools. I would prefer that you do this now but if you choose
not to, a bug to track this would be ok I suppose.
At this point I would prefer to address that issue with separate bug,
since I think it requires more thought about possible scenarios - e.g.
how to derive pool name in case mountpoint is not provided and
what approach to choose, so that it works for zones as well.
OK.
Thank you - I will file bug for this.
OK.
Hmm, the code suspicious then. If 'expire' is an empty string,
then will the call to set_expiration_date() see a "0" for the
second arg? i.e. the check at line 279
No - if 'expire' is configured as empty string in SC manifest, then
the second
arg of set_expiration_date() will be empty string as well. That
means, we will
end up calling usermod(1M) in following way:
usermod -e "" "$account"
That said, 'usermod -e' modifies 'expire' field in shadow(4) and
since I have
realized that it is set to empty string for created user account, the
call above
is actually no-op.
Based on that, I have added check for empty string for 'expire' as well
in order to skip the call in 'empty string' case.
If 'expire' is defined as "0" in SC manifest, the second arg of
set_expiration_date()
sees also "0" - in that case 'passwd -f' is called to expire password
immediately -
user is forced to enter new password at next login.
OK.
A similar comment for set_password() I guess. It seems
set_password() just assumes the second arg is usable.
I have tested with empty string and it seems to work fine - if
the 2nd arg of set_password() is empty string, 'pass' variable
in that function contains also empty string and nawk(1) does the
right job - e.g. sets 'password' in shadow(4) to empty string.
OK.
I have updated webrevs with the change above:
http://cr.opensolaris.org/~dambi/bug-15723-cr/
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/
The updated webrev looks ok.
thanks,
-ethan
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
