From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 19 Jul 2011 10:40:17 +0100 Subject: [Cluster-devel] [PATCH] mkfs: Remove duplicated code from verify_bsize() In-Reply-To: <20110718184433.GA3160@andromeda.usersys.redhat.com> References: <1310479579-12731-1-git-send-email-cmaiolino@redhat.com> <4E1C5C46.5050205@redhat.com> <20110718184433.GA3160@andromeda.usersys.redhat.com> Message-ID: <1311068417.2773.17.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 Mon, 2011-07-18 at 15:44 -0300, Carlos Maiolino wrote: > My apologies for my delay, I was on PTO. > > > are_you_sure() doesn't really belong in libgfs2 because it's user > > interface code and it has exit() calls (via die()). You might want > > to read bz408631 to see where libgfs2 is heading. > > > Gotcha will look. > > >+/*Function prototypes*/ > > > > Minor point, but I personally dislike comments like this. Everyone > > knows what a function prototype looks like :) > > > Indeed. I added it mainly to "make it clear" but I agree in do not add > this comment. > > > I'm not sure that it is worth using are_you_sure() here, as it opens > > the device again and calls check_dev_content() (which calls pipe() > > and then fork() ...), which is a lot more overhead than the few > > lines that you're removing here. Perhaps you could change > > are_you_sure to be more generic and move the checking into a > > separate function? > > > Looks like both (Steve and Andy) agrees to remove open() call from > are_you_sure() function, so, looks that's the best way to fix this up > and maybe as Andy suggestion set up another function to test device open > if necessary. > > My concern here btw, is if there is any problem in add the function prototype > to the beginning of the file. > > What you guys think? No, I can't see any problem with that, it sounds good to me, Steve.