[PATCH] add-shell, remove-shell: new applets
Tito
farmatito at tiscali.it
Wed Nov 3 07:04:15 UTC 2010
On Wednesday 03 November 2010 02:48:57 Denys Vlasenko wrote:
> On Wednesday 20 October 2010 17:30, Tito wrote:
> > 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
>
> Tito, you are turning update_passwd() into a Franken-function.
>
> It sings. It dances. It has so many uses it's hard to understand
> both the meaning of its parameters in all its many (nine!)
> possible ways to be called, and more importantly,
> it's hard to understand its source. Or rather, now it's even
> hardER to understand its source.
>
> Yes, this way add-shell and remove shell is smaller.
> Is it worth the reduced simplicity?
Hi,
I think yes it is worth it as update_passwd has all the machinery in place for
updating files in /etc that is duplicated in add/remove shell
and was duplicated (or missing) in most of the loginutils
at time it was cobbled together.
So if it does the safe/right thing for /etc/passwd /etc/group /etc/shadow
/etc/gshadow why not for /etc/shells. Wasn't a goal to reuse existing
code (proven to work). This is difficult without adding a minimum of
complexity to the code. In this case I just have added 12 lines
to update_passwd to do the work of about 70 lines of code in
add/remove shell.
My personal point of view about simplicity is that if I as a self-taught
programmer understand it, it must be simple code :-) ,
compared for example to the stuff in sha1 that I never
will fully understand lacking (or maybe having forgot about )
the needed mathematical background.
I agree with you that currently the name of the function in libbb
is misleading and maybe should be renamed in
bb_safe_update_files_in_etc_and_whatnot. :-)
But you are the boss....
Ciao,
Tito
>
> +
> +#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 */
>
>
More information about the busybox
mailing list