Hi,
while looking at the last commits to adduser I've spotted
some code that attracted my attention:
opt_complementary = "-1:?2:SD:u+";
if (sizeof(pw.pw_uid) == sizeof(int)) {
opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir,
&pw.pw_gecos, &pw.pw_shell, &usegroup, &pw.pw_uid);
} else {
unsigned uid;
opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir,
&pw.pw_gecos, &pw.pw_shell, &usegroup, &uid);
if (opts & OPT_UID) {
pw.pw_uid = uid;
}
}
I really was not able to understand the need for the double getopt32 call,
but maybe I'm just not smart enough.
Nonetheless:
1) u+ in opt_complementary means that the parameter
for this option is a nonnegative integer. It will be processed
with xatoi_positive() - allowed range is 0..INT_MAX.
2) even if sizeof(pw.pw_uid) is not sizeof(int) and we assume that it is bigger
sizeof (unsigned)
i think that our 0 to INT_MAX sized arg would always fit.
3) we use already the logic proposed by the fix in addgroup.
So I would simplify the code like:
opt_complementary = "-1:?2:SD:u+";
opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir, &pw.pw_gecos,
&pw.pw_shell, &usegroup, &pw.pw_uid);
As this seems to easy I supect I'm overlooking something obvious
and therefore hints, critics and improvements by the list members
are welcome. (Please have mercy!)
--- loginutils/adduser.c.orig 2013-12-21 12:52:52.000000000 +0100
+++ loginutils/adduser.c 2013-12-21 12:57:59.188565203 +0100
@@ -164,16 +164,11 @@ int adduser_main(int argc UNUSED_PARAM,
/* at least one and at most two non-option args */
/* disable interactive passwd for system accounts */
+ /* uid: the parameter for this option is a nonnegative integer,*/
+ /* allowed range is 0..INT_MAX.*/
opt_complementary = "-1:?2:SD:u+";
- if (sizeof(pw.pw_uid) == sizeof(int)) {
- opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir,
&pw.pw_gecos, &pw.pw_shell, &usegroup, &pw.pw_uid);
- } else {
- unsigned uid;
- opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir,
&pw.pw_gecos, &pw.pw_shell, &usegroup, &uid);
- if (opts & OPT_UID) {
- pw.pw_uid = uid;
- }
- }
+ opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir, &pw.pw_gecos,
&pw.pw_shell, &usegroup, &pw.pw_uid);
+
argv += optind;
pw.pw_name = argv[0];
I've spotted one more inconsistency in the function passwd_study(struct passwd
*p)
where in a comment we assume UID_T_MAX == INT_MAX
meanwhile in the code we do;
int max = UINT_MAX;
This is fixed as:
/* recoded such that the uid may be passed in *p */
static void passwd_study(struct passwd *p)
{
- int max = UINT_MAX;
+ int max = INT_MAX;
if (getpwnam(p->pw_name)) {
bb_error_msg_and_die("%s '%s' in use", "user", p->pw_name);
Ciao,
Tito
Simplify adduser.
Signed-off-by: Tito Ragusa <[email protected]>
--- loginutils/adduser.c.orig 2013-12-21 12:52:52.000000000 +0100
+++ loginutils/adduser.c 2013-12-21 15:14:11.777219649 +0100
@@ -41,7 +41,7 @@
/* recoded such that the uid may be passed in *p */
static void passwd_study(struct passwd *p)
{
- int max = UINT_MAX;
+ int max = INT_MAX;
if (getpwnam(p->pw_name)) {
bb_error_msg_and_die("%s '%s' in use", "user", p->pw_name);
@@ -164,16 +164,11 @@ int adduser_main(int argc UNUSED_PARAM,
/* at least one and at most two non-option args */
/* disable interactive passwd for system accounts */
+ /* option uid: the parameter for this option is a nonnegative integer,*/
+ /* allowed range is 0..INT_MAX.*/
opt_complementary = "-1:?2:SD:u+";
- if (sizeof(pw.pw_uid) == sizeof(int)) {
- opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir, &pw.pw_gecos, &pw.pw_shell, &usegroup, &pw.pw_uid);
- } else {
- unsigned uid;
- opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir, &pw.pw_gecos, &pw.pw_shell, &usegroup, &uid);
- if (opts & OPT_UID) {
- pw.pw_uid = uid;
- }
- }
+ opts = getopt32(argv, "h:g:s:G:DSHu:", &pw.pw_dir, &pw.pw_gecos, &pw.pw_shell, &usegroup, &pw.pw_uid);
+
argv += optind;
pw.pw_name = argv[0];
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
