From mboxrd@z Thu Jan 1 00:00:00 1970 From: Carlos Maiolino Date: Mon, 18 Jul 2011 15:44:33 -0300 Subject: [Cluster-devel] [PATCH] mkfs: Remove duplicated code from verify_bsize() In-Reply-To: <4E1C5C46.5050205@redhat.com> References: <1310479579-12731-1-git-send-email-cmaiolino@redhat.com> <4E1C5C46.5050205@redhat.com> Message-ID: <20110718184433.GA3160@andromeda.usersys.redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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? -- --Carlos