Re: possible mountroot regression

on 14/10/2011 18:54 Arnaud Lacombe said the following:
> Andry Gapon wrote:
>> Simple: revert to the previous behavior.  If a user enters incorrect device name
>> (i.e. root mounting fails), then return back to the prompt instead of panicing.
> That should do the job.
> 
>  - Arnaud
> 
> ---
>  sys/kern/vfs_mountroot.c |   45 +++++++++++++++++++++++----------------------
>  1 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/sys/kern/vfs_mountroot.c b/sys/kern/vfs_mountroot.c
> index ccbcb33..ae3ffa7 100644
> --- a/sys/kern/vfs_mountroot.c
> +++ b/sys/kern/vfs_mountroot.c
> _at__at_ -481,28 +481,29 _at__at_ parse_dir_ask(char **conf)
>  	printf("\n");
>  	printf("  ?               List valid disk boot devices\n");
>  	printf("  .               Yield 1 second (for background tasks)\n");
> -	printf("  <empty line>    Abort manual input\n");
> +	printf("  x               Abort manual input)\n");
> +
> +	do {
> +		error = EINVAL;
> +		printf("\nmountroot> ");
> +		gets(name, sizeof(name), GETS_ECHO);
> +		if (name[0] == '?') {
> +			printf("\nList of GEOM managed disk devices:\n  ");
> +			g_dev_print();
> +			continue;
> +		}
> +		if (name[0] == '.') {
> +			pause("rmask", hz);
> +			continue;
> +		}
> +		if (name[0] == 'x' && name[1] == '\0')
> +			break;
> +		mnt = name;
> +		error = parse_mount(&mnt);
> +		if (error < 0)
> +			printf("Invalid specification.\n");
> +	} while (error != 0);
>  
> - again:
> -	printf("\nmountroot> ");
> -	gets(name, sizeof(name), GETS_ECHO);
> -	if (name[0] == '\0')
> -		return (0);
> -	if (name[0] == '?') {
> -		printf("\nList of GEOM managed disk devices:\n  ");
> -		g_dev_print();
> -		goto again;
> -	}
> -	if (name[0] == '.') {
> -		pause("rmask", hz);
> -		goto again;
> -	}
> -	mnt = name;
> -	error = parse_mount(&mnt);
> -	if (error == -1) {
> -		printf("Invalid specification.\n");
> -		goto again;
> -	}
>  	return (error);
>  }
>  

Arnaud,

I like how your change fixes the regression and improves code style.
As you've said, the 'x' change is unrelated.  I like it, but it needs to be
discussed and committed separately.

Marcel,

what do you think?
Would you be able to commit a variant of this patch sans the 'x' part?

-- 
Andriy Gapon