On Wednesday 20 October 2010 16:36:48 Denys Vlasenko wrote:
> 2010/10/20 Alexander Shishkin <[email protected]>:
> > On Wed, Oct 20, 2010 at 01:26:24 +0200, Denys Vlasenko wrote:
> >> On Wed, Oct 20, 2010 at 2:56 AM, Alexander Shishkin <[email protected]>
> >> wrote:
> >> > How about this? (I left locking in place but applied other changes)
> >> >
> >> > function old new delta
> >> > add_shell_main - 497 +497
> >> > .rodata 144696 144795 +99
> >> > packed_usage 27078 27114 +36
> >> > applet_names 2259 2282 +23
> >> > applet_main 2672 2688 +16
> >> > applet_nameofs 668 672 +4
> >> > applet_install_loc 167 168 +1
> >> > ------------------------------------------------------------------------------
> >> > (add/remove: 2/0 grow/shrink: 6/0 up/down: 676/0) Total: 676
> >> > bytes
> >>
> >> 676 bytes is far too much for a simple pass over a text file.
> >>
> >> How about this?
> >>
> >> http://git.busybox.net/busybox/commit/?id=5be79ff27a5852567a9bdec80d67b061ad828290
> >
> > Well, the list version is more readable, imo.
>
> It's twice as big.
>
> > Also, splitting add and remove
> > into separate options seems quite pointless.
>
> What if I want one-applet version of add-shell?
> (.config with only one enabled has optimizations
> which skip applet table search, and therefore
> there is no applet table in the first place. etc...)
>
> > And I'm totally missing the point
> > of using stdout, but I'm sure there must be a good reason.
>
> How can you do O_EXCL open with fopen?
>
> > Otherwise seems fine,
> > thanks for spending time on this.
>
Hi,
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!
Enjoy!
Ciao,
Tito
#include "libbb.h"
#define SHELLS_FILE "/etc/shells"
int add_remove_shell_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
{
if (argc < 2)
bb_show_usage();
argv++;
do {
if (update_passwd(SHELLS_FILE, *argv, "", NULL) < 0)
return EXIT_FAILURE;
} while (*++argv);
return EXIT_SUCCESS;
}
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 16:51:43.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,6 +177,18 @@
line = xmalloc_fgetline(old_fp);
if (!line) /* EOF/error */
break;
+#if ENABLE_ADD_SHELL || ENABLE_REMOVE_SHELL
+ 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
if (strncmp(name_colon, line, user_len) != 0) {
fprintf(new_fp, "%s\n", line);
goto next;
@@ -247,7 +269,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
