From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 12 Jul 2011 15:26:14 +0100 Subject: [Cluster-devel] [PATCH] mkfs: Remove duplicated code from verify_bsize() In-Reply-To: <1310479579-12731-1-git-send-email-cmaiolino@redhat.com> References: <1310479579-12731-1-git-send-email-cmaiolino@redhat.com> Message-ID: <1310480774.2731.10.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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.