From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jes Sorensen Subject: Re: [PATCH 1/4] mdadm/util:integrate stat operations into one utility Date: Fri, 14 Apr 2017 11:20:29 -0400 Message-ID: References: <20170401125145.14440-1-zlliu@suse.com> <20170401125145.14440-2-zlliu@suse.com> <0f6a630a-c53a-090e-67c3-8132f6d7b08e@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <0f6a630a-c53a-090e-67c3-8132f6d7b08e@suse.com> Sender: linux-raid-owner@vger.kernel.org To: Liu Zhilong Cc: linux-raid@vger.kernel.org List-Id: linux-raid.ids On 04/14/2017 06:14 AM, Liu Zhilong wrote: > > > On 04/05/2017 11:42 PM, jes.sorensen@gmail.com wrote: >> Zhilong Liu writes: [snip] >> For a function like this, lets name it better md_is_blkdev() > > How about return the devid after checking? Because always need the > stb.st_rdev to parse > the major and minor number. Although "util.c" has "devnm2devid" to > gather the devid via > to devnm, but it's convenient to return devid when check the blkdev with > absolute path. > would you mind the function like this? > > // returns dev-id when success, return 0 when failure > dev_t stat_md_is_blkdev(char *dev) > { > struct stat stb; > > if (stat(dev, &stb) != 0) { > pr_err("stat failed for %s: %s\n", dev, strerror(errno)); > return 0; > } > if ((S_IFMT & stb.st_mode) != S_IFBLK) { > pr_err("%s is not a block device.\n", dev); > return 0; > } > return stb.st_rdev; > } I am generally wary of too many smart handlers, but I think it makes some sense here to de-duplicate the repeated code. That said, your function would ditch the error information, and the caller wouldn't know why it failed. If you made it more like this and return the error code, plus stick the major/minor number into rdev, if one is provided: int stat_md_is_blkdev(char *devname, *dev_t rdev) { } Jes