On Wednesday 20 October 2010 17:14:09 Baruch Siach wrote:
> Hi Tito,
>
> On Wed, Oct 20, 2010 at 05:03:38PM +0200, Tito wrote:
>
> [snip]
>
> > just for fun a different approach to the add/remove shell problem.
> > This is really small. Bloat-o-meter says:
> >
> > debian:~/Desktop/test/busybox$ scripts/bloat-o-meter busybox_old
> > busybox_unstripped
> > function old new delta
> > update_passwd 1273 1335 +62
> > dont_add 2 - -2
> > .rodata 135419 135406 -13
> > add_remove_shell_main 259 62 -197
> > ------------------------------------------------------------------------------
> > (add/remove: 0/1 grow/shrink: 1/2 up/down: 62/-212) Total: -150
> > bytes
> >
> > It is a little tested and seems to work fine for me.
> > It also adds argc check and usage message.
> > Hints and critics on how to shrink it even more are welcome!
>
> [snip]
>
> > @@ -167,6 +177,18 @@
> > line = xmalloc_fgetline(old_fp);
> > if (!line) /* EOF/error */
> > break;
> > +#if ENABLE_ADD_SHELL || ENABLE_REMOVE_SHELL
>
> This #if seems redundant to me, and it makes the code less readable. The
> compiler should optimize this code section away when the (ADD_SHELL ||
> REMOVE_SHELL) expression is 0 at compile time.
>
> baruch
>
> > + if (ADD_SHELL || REMOVE_SHELL) {
> > + /* remove colon */
> > + name_colon[user_len - 1] = '\0';
> > + if (strcmp(name_colon, line) != 0) {
> > + fprintf(new_fp, "%s\n", line);
> > + goto next;
> > + } else if (REMOVE_SHELL) {
> > + changed_lines++;
> > + }
> > + } else
> > +#endif
>
>
Hi,
was not sure if it could be removed and was to lazy to test.
Fixed.
Thanks for your hint.
Attached v2 of thr patch.
Ciao,
Tito
Signed-off-by: Tito Ragusa <[email protected]>
--- loginutils/add-remove-shell.c.orig 2010-10-20 14:41:23.000000000 +0200
+++ loginutils/add-remove-shell.c 2010-10-20 16:35:57.000000000 +0200
@@ -40,88 +40,16 @@
#define SHELLS_FILE "/etc/shells"
-#define REMOVE_SHELL (ENABLE_REMOVE_SHELL && (!ENABLE_ADD_SHELL || applet_name[0] == 'r'))
-#define ADD_SHELL (ENABLE_ADD_SHELL && (!ENABLE_REMOVE_SHELL || applet_name[0] == 'a'))
-
-/* NB: we use the _address_, not the value, of this string
- * as a "special value of pointer" in the code.
- */
-static const char dont_add[] ALIGN1 = "\n";
-
int add_remove_shell_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
{
- FILE *orig_fp;
- char *orig_fn;
- char *new_fn;
-
+ if (argc < 2)
+ bb_show_usage();
argv++;
-
- orig_fn = xmalloc_follow_symlinks(SHELLS_FILE);
- if (!orig_fn)
- return EXIT_FAILURE;
- orig_fp = fopen_for_read(orig_fn);
-
- new_fn = xasprintf("%s.tmp", orig_fn);
- xmove_fd(xopen(new_fn, O_WRONLY | O_CREAT | O_EXCL), STDOUT_FILENO);
-
- /* TODO:
- struct stat sb;
- fstat(fileno(orig_fp), &sb);
- xfchown(STDOUT_FILENO, sb.st_uid, sb.st_gid);
- xfchmod(STDOUT_FILENO, sb.st_mode);
- */
-
- if (orig_fp) {
- /* Copy old file, possibly skipping removed shell names */
- char *line;
- while ((line = xmalloc_fgetline(orig_fp)) != NULL) {
- char **cpp = argv;
- while (*cpp) {
- if (strcmp(*cpp, line) == 0) {
- /* Old file has this shell name */
- if (REMOVE_SHELL) {
- /* we are remove-shell */
- /* delete this name by not copying it */
- goto next_line;
- }
- /* we are add-shell */
- /* mark this name as "do not add" */
- *cpp = (char*)dont_add;
- }
- cpp++;
- }
- /* copy shell name from old to new file */
- printf("%s\n", line);
- next_line:
- free(line);
- }
- if (ENABLE_FEATURE_CLEAN_UP)
- fclose(orig_fp);
- }
-
- if (ADD_SHELL) {
- char **cpp = argv;
- while (*cpp) {
- if (*cpp != dont_add)
- printf("%s\n", *cpp);
- cpp++;
- }
- }
-
- /* Ensure we wrote out everything */
- if (fclose(stdout) != 0) {
- xunlink(new_fn);
- bb_perror_msg_and_die("%s: write error", new_fn);
- }
-
- /* Small hole: if rename fails, /etc/shells.tmp is not removed */
- xrename(new_fn, orig_fn);
-
- if (ENABLE_FEATURE_CLEAN_UP) {
- free(orig_fn);
- free(new_fn);
- }
+ do {
+ if (update_passwd(SHELLS_FILE, *argv, "", NULL) < 0)
+ return EXIT_FAILURE;
+ } while (*++argv);
return EXIT_SUCCESS;
}
--- libbb/update_passwd.c.orig 2010-09-22 16:14:08.000000000 +0200
+++ libbb/update_passwd.c 2010-10-20 17:28:16.000000000 +0200
@@ -62,11 +62,21 @@
only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd
+ 8) add a shell: update_passwd(SHELLS_FILE, SHELL, "", NULL)
+ only if CONFIG_ADD_SHELL=y and applet_name[3] == '-' like in add-shell
+
+ 9) remove a shell: update_passwd(SHELLS_FILE, SHELL, "", NULL)
+ only if CONFIG_REMOVE_SHELL=y and applet_name[6] == '-' like in remove-shell
+
This function does not validate the arguments fed to it
so the calling program should take care of that.
Returns number of lines changed, or -1 on error.
*/
+
+#define REMOVE_SHELL (ENABLE_REMOVE_SHELL && (!ENABLE_ADD_SHELL || applet_name[6] == '-'))
+#define ADD_SHELL (ENABLE_ADD_SHELL && (!ENABLE_REMOVE_SHELL || applet_name[3] == '-'))
+
int FAST_FUNC update_passwd(const char *filename,
const char *name,
const char *new_passwd,
@@ -167,7 +177,16 @@
line = xmalloc_fgetline(old_fp);
if (!line) /* EOF/error */
break;
- if (strncmp(name_colon, line, user_len) != 0) {
+ if (ADD_SHELL || REMOVE_SHELL) {
+ /* remove colon */
+ name_colon[user_len - 1] = '\0';
+ if (strcmp(name_colon, line) != 0) {
+ fprintf(new_fp, "%s\n", line);
+ goto next;
+ } else if (REMOVE_SHELL) {
+ changed_lines++;
+ }
+ } else if (strncmp(name_colon, line, user_len) != 0) {
fprintf(new_fp, "%s\n", line);
goto next;
}
@@ -247,7 +266,7 @@
bb_error_msg("can't find %s in %s", member, filename);
}
#endif
- if ((ENABLE_ADDUSER || ENABLE_ADDGROUP)
+ if ((ENABLE_ADDUSER || ENABLE_ADDGROUP || ENABLE_ADD_SHELL)
&& applet_name[0] == 'a' && !member
) {
/* add user or group */
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
