Re: [PATCH 1/4] mdadm/util:integrate stat operations into one utility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 04/14/2017 11:20 PM, Jes Sorensen wrote:
On 04/14/2017 06:14 AM, Liu Zhilong wrote:


On 04/05/2017 11:42 PM, jes.sorensen@xxxxxxxxx wrote:
Zhilong Liu <zlliu@xxxxxxxx> 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)
{
}


I want to integrate two situations into one function.
1. only check the 'dev'(such as /dev/loop2) whether or not is block device.
2. optionally return the rdev-id after checking the block device.
this sample "int stat_md_is_blkdev(char *devname, *dev_t rdev)" is smart for the situation 2. but for situation 1, the second parameter is a little waste.

how about like this?
dev_t stat_md_is_blkdev(char *devname, bool require_rdev)
{
    struct stat stb;

       if (stat(devname, &stb) != 0) {
               printf("stat failed for %s.\n", devname);
               return 1;
       }
       if ((S_IFMT & stb.st_mode) != S_IFBLK) {
               printf("%s is not a block device.\n", devname);
               return 1;
       }
       if (require_rdev)
          return stb.st_rdev;
    return 0;
}

the caller would be like this,
situation 1:
char dev[20] = "/dev/loop2";
if (stat_md_is_blkdev(dev, false))
  return 1;

situation 2:
dev_t st_rdev;
char dev[20] = "/dev/loop2";
st_rdev = stat_md_is_blkdev(dev, true);
if (st_rdev == 1)
  return 1;


Thanks very much,
-Zhilong

Jes



--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux