[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    [PATCH] addgroup code cleanup, size reduction and feature enhancement
From:       Tito <farmatito () tiscali ! it>
Date:       2007-03-27 20:44:04
Message-ID: 200703272244.04553.farmatito () tiscali ! it
[Download RAW message or body]

Hi to all,

this patch cleans up addgroup, reduces the size, removes a crippled feature,
and re-adds it as optional fully functional feature.

1) code cleanup and size reduction (with make defconfig; make):

scripts/bloat-o-meter busybox_old busybox_unstripped
function                                             old     new   delta
packed_usage                             22068   22058     -10
bb_internal_fgetgrent_r                      51          -      -51
.rodata                                      124387 124323     -64
addgroup_main                               380       237   -143
------------------------------------------------------------------------------
(add/remove: 0/1 grow/shrink: 0/3 up/down: 0/-268)           Total: -268 bytes
 
The size reduction was achieved through an almost complete rewrite of
the functions group_study and addgroup and a tighter integration
with libbb.

2) crippled function removal:
the current addgroup applet when called with 2 arguments
permits only to add an existing user to an still to be created group.
This seems to me a crippled behavior as it could be used just one time
at group creation time: 

root@localhost:~/Desktop/busybox.new# ./busybox_old addgroup prova tito
root@localhost:~/Desktop/busybox.new# ./busybox_old addgroup prova dina
addgroup: prova: group already in use

3) the above mentioned feature was re-added as optional
    function enabled by ENABLE_FEATURE_ADDUSER_TO_GROUP.
    Now    if  called  with two non-option arguments, addgroup will add an existing
    user to an existing group like the full blown addgroup does (at least on my system).

	addgroup dina prova
	Adding user `dina' to group `prova'...
	Done.

	./busybox_unstripped addgroup prova
	./busybox_unstripped addgroup dina prova
	./busybox_unstripped addgroup tito prova

	prova:x:1004:dina,tito
	prova:!::dina,tito

Bloat-o-meter says:

scripts/bloat-o-meter busybox_old busybox_unstripped
function                                             old       new   delta
add_user_to_group                                -       237  +237
packed_usage                               22068  22058      -10
addgroup_main                                 380     354       -26
.rodata                                        124387 124355     -32
bb_internal_fgetgrent_r                       51            -     -51
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 0/3 up/down: 237/-119)          Total: 118 bytes

This feature is turned of by default.

This patch is tested and seems to work as expected.
Comments and critics are as always welcome.
Please apply if you like it.

Ciao,
Tito

["addgroup.patch" (text/x-diff)]

--- loginutils/addgroup_orig.c	2007-03-22 22:58:42.000000000 +0100
+++ loginutils/addgroup.c	2007-03-27 22:32:16.000000000 +0200
@@ -1,9 +1,10 @@
 /* vi: set sw=4 ts=4: */
 /*
- * addgroup - add users to /etc/passwd and /etc/shadow
+ * addgroup - add groups to /etc/group and /etc/gshadow
  *
  * Copyright (C) 1999 by Lineo, inc. and John Beppu
  * Copyright (C) 1999,2000,2001 by John Beppu <beppu@codepoet.org>
+ * Copyright (C) 2007 by Tito Ragusa <farmatito@tiscali.it>
  *
  * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
  *
@@ -11,48 +12,40 @@
 
 #include "busybox.h"
 
-/* make sure gr_name isn't taken, make sure gid is kosher
- * return 1 on failure */
-static int group_study(struct group *g)
+static void xgroup_study(struct group *g)
 {
 	enum { max = 65000 };
-	FILE *etc_group;
-	gid_t desired;
-	/* Using _r function to avoid static buffers pulled in */
-	char buffer[256];
-	struct group grp;
-	struct group *result;
-
-	etc_group = xfopen(bb_path_group_file, "r");
-
-	/* make sure gr_name isn't taken, make sure gid is kosher */
-	desired = g->gr_gid;
-	while (!fgetgrent_r(etc_group, &grp, buffer, sizeof(buffer), &result)) {
-		if ((strcmp(grp.gr_name, g->gr_name)) == 0) {
-			bb_error_msg_and_die("%s: group already in use", g->gr_name);
-		}
-		if ((desired) && grp.gr_gid == desired) {
-			bb_error_msg_and_die("%d: gid already in use",
-							  desired);
+	/* Use a particular gid. */
+	int desired = (g->gr_gid > 0);
+
+	/* Make sure gr_name is unused */
+	if (getgrnam(g->gr_name)) {
+		goto error;
+	}
+
+	/* Check if the desired gid is free or 
+	   find the first free one */
+	do {
+		if (g->gr_gid == max) { /* out of bounds: exit */
+			bb_error_msg_and_die("no gids left");
 		}
-		if ((grp.gr_gid > g->gr_gid) && (grp.gr_gid < max)) {
-			g->gr_gid = grp.gr_gid;
+		if (!getgrgid(g->gr_gid)) {
+			return; /* ok */
 		}
-	}
-	if (ENABLE_FEATURE_CLEAN_UP)
-		fclose(etc_group);
+		if (desired) { /* the desired gid is already in use: exit */
+			g->gr_name = itoa(g->gr_gid);
+			goto error;
+		}
+		g->gr_gid++;
+	} while (1);
 
-	/* gid */
-	g->gr_gid++;
-	if (desired) {
-		g->gr_gid = desired;
-	}
-	/* return 1; */
-	return 0;
+error:
+	/* exit */
+	bb_error_msg_and_die("%s: already in use", g->gr_name);
 }
 
 /* append a new user to the passwd file */
-static int addgroup(char *group, gid_t gid, const char *user)
+static void new_group(char *group, gid_t gid)
 {
 	FILE *file;
 	struct group gr;
@@ -60,16 +53,14 @@
 	/* make sure gid and group haven't already been allocated */
 	gr.gr_gid = gid;
 	gr.gr_name = group;
-	if (group_study(&gr))
-		return 1;
+	xgroup_study( &gr);
 
 	/* add entry to group */
 	file = xfopen(bb_path_group_file, "a");
 	/* group:passwd:gid:userlist */
-	fprintf(file, "%s:%s:%d:%s\n", group, "x", gr.gr_gid, user);
+	fprintf(file, "%s:x:%d:\n", group, gr.gr_gid);
 	if (ENABLE_FEATURE_CLEAN_UP)
 		fclose(file);
-
 #if ENABLE_FEATURE_SHADOWPASSWDS
 	file = fopen_or_warn(bb_path_gshadow_file, "a");
 	if (file) {
@@ -78,15 +69,60 @@
 			fclose(file);
 	}
 #endif
+}
 
-	/* return 1; */
-	return 0;
+#if ENABLE_FEATURE_ADDUSER_TO_GROUP
+static void add_user_to_group(char **args,
+		const char *path,
+		FILE *(*fopen_func)(const char *fileName, const char *mode))
+{
+	char *line;
+	int len = strlen(args[1]);
+	llist_t *plist = NULL;
+	FILE *group_file;
+
+	group_file = fopen_func(path, "r");
+
+	if (!group_file) return;
+
+	while ((line = xmalloc_getline(group_file))) {
+		/* Find the group */
+		if (!strncmp(line, args[1], len)
+		&& line[len] == ':'
+		) {
+			/* Add the new user */
+			line = xasprintf("%s%s%s", line,
+						last_char_is(line, ':') ? "" : ",",
+						args[0]);
+		}
+		llist_add_to_end(&plist, line);
+	}
+
+	if (ENABLE_FEATURE_CLEAN_UP) {
+		fclose(group_file);
+		group_file = fopen_func(path, "w");
+		while ((line = llist_pop(&plist))) {
+			if (group_file)
+				fprintf(group_file, "%s\n", line);
+			free(line);
+		}
+		if (group_file)
+			fclose(group_file);
+	} else {
+		group_file = fopen_func(path, "w");
+		if (group_file)
+			while ((line = llist_pop(&plist)))
+				fprintf(group_file, "%s\n", line);
+	}
 }
+#endif
 
 /*
  * addgroup will take a login_name as its first parameter.
  *
  * gid can be customized via command-line parameters.
+ * If  called with two non-option arguments, addgroup
+ * will add an existing user to an existing group.
  */
 int addgroup_main(int argc, char **argv);
 int addgroup_main(int argc, char **argv)
@@ -101,11 +137,36 @@
 	}
 	/* move past the commandline options */
 	argv += optind;
+	argc -= optind;
 
 	/* need to be root */
 	if (geteuid()) {
 		bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
 	}
 
-	return addgroup(argv[0], gid, argv[1] ? argv[1] : "");
+#if ENABLE_FEATURE_ADDUSER_TO_GROUP
+	if (argc == 2) {
+		struct group *gr;
+		
+		/* check if group and user exist */
+		xuname2uid(argv[0]); /* unknown user: exit */
+		xgroup2gid(argv[1]); /* unknown group: exit */
+		/* check if user is already in this group */
+		gr = getgrnam(argv[1]);
+		for (; *(gr->gr_mem) != NULL; (gr->gr_mem)++) {
+			if (!strcmp(argv[0], *(gr->gr_mem))) {
+				/* user is already in group: do nothing */
+				return EXIT_SUCCESS;
+			}
+		}
+		add_user_to_group(argv, bb_path_group_file, xfopen);
+#if ENABLE_FEATURE_SHADOWPASSWDS
+		add_user_to_group(argv, bb_path_gshadow_file, fopen_or_warn);
+#endif /* ENABLE_FEATURE_SHADOWPASSWDS */
+	} else
+#endif /* ENABLE_FEATURE_ADDUSER_TO_GROUP */
+		new_group(argv[0], gid);
+
+	/* Reached only on success */
+	return EXIT_SUCCESS;
 }
--- loginutils/Config.in_orig	2007-01-23 21:44:12.000000000 +0100
+++ loginutils/Config.in	2007-03-27 22:34:19.000000000 +0200
@@ -59,6 +59,15 @@
 	help
 	  Utility for creating a new group account.
 
+config FEATURE_ADDUSER_TO_GROUP
+	bool "Support for adding users to groups"
+	default n
+	depends on ADDGROUP
+	help
+	  If  called  with two non-option arguments,
+	  addgroup will add an existing user to an
+	  existing group.
+
 config DELGROUP
 	bool "delgroup"
 	default n
--- include/usage.h_orig	2007-03-27 21:03:13.000000000 +0200
+++ include/usage.h	2007-03-27 22:39:02.000000000 +0200
@@ -12,9 +12,9 @@
 #define __BB_USAGE_H__
 
 #define addgroup_trivial_usage \
-       "[-g GID] group_name [user_name]"
+       "[-g GID]"USE_FEATURE_ADDUSER_TO_GROUP(" [user_name]")" group_name"
 #define addgroup_full_usage \
-       "Add a group to the system" \
+       "Add a group to the system"USE_FEATURE_ADDUSER_TO_GROUP(" or add an user to a group") \
        "\n\nOptions:\n" \
        "	-g GID	Specify gid"
 


_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic