On Fri, Dec 06, 2019 at 04:03:20PM +0900, Naohiro Aota wrote:
> >> +#define BTRFS_SUPER_INFO_SIZE 4096
> >
> >I believe that 4K is very widely used constant.
> >Are you sure that it needs to introduce some
> >additional constant? Especially, it looks slightly
> >strange to see the BTRFS specialized constant.
> >Maybe, it needs to generalize the constant?
>
> I don't think so...
>
> I think it is better to define BTRFS_SUPER_INFO_SIZE here. This is an
> already defined constant in btrfs-progs and this is key value to
> calculate the last superblock location. I think it's OK to define
> btrfs local constant in btrfs.c file...
I agree, the named constant makes the meaning more clear. In the code
where it's used:
> >> + if (ret != -ENOENT) {
> >> + if (wp == zones[0].start << SECTOR_SHIFT)
> >> + wp = (zones[1].start + zones[1].len) <<
> >> SECTOR_SHIFT;
> >> + wp -= BTRFS_SUPER_INFO_SIZE;
> >> + }
If there's just
wp -= 4096;
it's a magic constant out of nowhere. As pointed out, it's defined only
in btrfs.c so it does not pollute namespace in libblkid.
