[Cluster-devel] [PATCH] mkfs: Remove duplicated code from verify_bsize()

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Tue, 2011-07-12 at 11:06 -0300, Carlos Maiolino wrote:
> Although are_you_sure() function adds some little extra overhead due some
> extra checks, use this function instead of duplicate code is worth.
> The extra security checks and the additional overhead will not be
> noticed by the user.
> 
> This is the first try of this patch, but I think the good way to do that
> is to move are_you_sure() and some related functions to libgfs2.
> ---
This looks like another bit of code that should be simplified. Why does
are_you_sure() open the device like that I wonder? Its just been opened
immediately above that in the code, so we can remove that opening of the
device, it does nothing useful.

>  gfs2/mkfs/main_mkfs.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
> index b0bb6e3..315f191 100644
> --- a/gfs2/mkfs/main_mkfs.c
> +++ b/gfs2/mkfs/main_mkfs.c
> @@ -26,6 +26,9 @@
>  #include "libgfs2.h"
>  #include "gfs2_mkfs.h"
>  
> +/*Function prototypes*/
> +static void are_you_sure(struct gfs2_sbd *sdp);
> +
>  int discard = 1;
>  
>  /**
> @@ -317,14 +320,7 @@ static void verify_bsize(struct gfs2_sbd *sdp)
>  		if (sdp->override)
>  			return;
>  
> -		printf( _("\nAre you sure you want to proceed? [y/n] "));
> -		if(!fgets(input, 32, stdin))
> -			die( _("unable to read from stdin\n"));
> -
> -		if (input[0] != 'y')
> -			die( _("aborted\n"));
> -		else
> -			printf("\n");
> +		are_you_sure(sdp);
>  	}
>  }
>  

This is a change of behaviour. Do we really want to run
check_dev_content() in this case? I suspect not. This is something which
could usefully be made into common code (the "are you sure" bit) but
please don't move it into libgfs2 as we are trying to keep all user
interface code out of there. A useful thing to do would be to work out
how to get at the glibc translations for useful phrases, such as "yes"
and "no" and rewrite the function using those,

Steve.