check_login in loginutils/login.c (was Re: [BusyBox] Re: login buggy also in 1.00pre5)

[Tito]

On Tuesday 06 January 2004 23:19, Friedrich Lobenstock wrote:
> Charlie Brady wrote on 06.01.2004 23:22 MET:
> > On Tue, 6 Jan 2004, Tito wrote:
> >>>>	if ( check_nologin ( pw-> pw_uid == 0 ))
> >>>>		return EXIT_FAILURE;
> >
> > ...
> >
> >>maybe something like :
> >> 	if ( check_nologin ( (pw-> pw_uid == 0) ) == 1)
> >> 		return EXIT_FAILURE;
> >>would be easier to understand for the non-gurus. ;-)
> >
> > But that might mean something different (the internals of
> > check_nologin() will say).
> >
> > Would you be happier with
> >
> > if (check_nologin(!pw->pw_uid))
> >     return EXIT_FAILURE;
>
> How about the patch below? It calls check_nologin for a user-id
> which I think is more clear and let me show you that we even
> reduce code size:
>
> # sed -e "/^$/d" -e "/^#/d" .config
> HAVE_DOT_CONFIG=y
> CONFIG_FEATURE_BUFFERS_GO_IN_BSS=y
> CONFIG_FEATURE_VERBOSE_USAGE=y
> CONFIG_FEATURE_DEVPTS=y
> EXTRA_CFLAGS_OPTIONS=""
> PREFIX="./_install"
> CONFIG_LOGIN=y
> CONFIG_FEATURE_SECURETTY=y
> CONFIG_FEATURE_SH_IS_NONE=y
>
> * without this patch applied (binary stripped):
>
>    # ls -al busybox-1.00-pre5/busybox
>    -rwxr-xr-x    1 fl       users       13424 2004-01-07 00:09
> busybox-1.00-pre5/busybox*
>
> * with this patch applied (binary stripped):
>
>    # ls -al busybox-1.00-pre5/busybox
>    -rwxr-xr-x    1 fl       users       13392 2004-01-07 00:13
> busybox-1.00-pre5/busybox*
>
>
> Difference: 32 Bytes (because we do one comparision less than before)
> [ note: result valid for the CRIS architecture, numbers might differ on
> other arch ]
>
> --- busybox-1.00-pre5/loginutils/login.c.org	2004-01-06 23:33:24.000000000
> +0100 +++ busybox-1.00-pre5/loginutils/login.c	2004-01-06
> 23:35:01.000000000 +0100 @@ -37,7 +37,7 @@
>   #define USERNAME_SIZE 32
>
>
> -static int check_nologin ( int amroot );
> +static int check_nologin ( int uid );
>
>   #if defined CONFIG_FEATURE_SECURETTY
>   static int check_tty ( const char *tty );
> @@ -217,7 +217,7 @@
>   	}
>
>   	alarm ( 0 );
> -	if ( check_nologin ( pw-> pw_uid == 0 ))
> +	if ( check_nologin ( pw-> pw_uid ))
>   		return EXIT_FAILURE;
>
>   #ifdef CONFIG_FEATURE_U_W_TMP
> @@ -315,7 +315,7 @@
>   }
>
>
> -static int check_nologin ( int amroot )
> +static int check_nologin ( int uid )
>   {
>   	if ( access ( bb_path_nologin_file, F_OK ) == 0 ) {
>   		FILE *fp;
> @@ -330,7 +330,7 @@
>   		} else {
>   			puts ( "\r\nSystem closed for routine maintenance.\r" );
>   		}
> -		if ( !amroot )
At this point to be coherent rather than writing a comment do
		if(uid!=0)
> +		if ( uid ) /* !root */
>   			return 1;
>
>   		puts ( "\r\n[Disconnect bypassed -- root login allowed.]\r" );

Looks easier to understand at the first glance...........
Tito