[parted-devel] FYI, preparation for 2048-byte sector sizes across the board

[Jim Meyering]

Otavio Salvador <otavio at debian.org> wrote:

> Jim Meyering <jim at meyering.net> writes:
>
>> Otavio Salvador <otavio at debian.org> wrote:
>>
>>> Jim Meyering <jim at meyering.net> writes:
>>>
>>>> diff --git a/libparted/labels/bsd.c b/libparted/labels/bsd.c
>>>> index 747a9c5..26a8b60 100644
>>>> --- a/libparted/labels/bsd.c
>>>> +++ b/libparted/labels/bsd.c
>>>> @@ -141,30 +142,45 @@ alpha_bootblock_checksum (char *boot) {
>>> <...>
>>>>  static int
>>>>  bsd_probe (const PedDevice *dev)
>>>>  {
>>>> -	char		boot[512];
>>>> -	BSDRawLabel	*label;
>>>> +	BSDRawLabel	*partition;
>>>>
>>>>  	PED_ASSERT (dev != NULL, return 0);
>>>>
>>>> -        if (dev->sector_size != 512)
>>>> +        if (dev->sector_size < 512)
>>>>                  return 0;
>>>
>>> Shouldn't it be:
>>>            if (dev->sector_size%512 != 0)
>>>                    return 0;
>>
>> Thanks for looking at it, but no.
>>
>> First, your replacement would mistakenly allow a dev->sector_size value
>> of 0 through.  More importantly, there is already plenty of code (e.g.,
>> in linux.c) that ensures it is a multiple of 512, so it would be redundant
>> to check for that in each label/*.c's *_probe function.
>
> Well but then we should "trust" the sector_size and continue. Maybe we
> ought to have a static method to check it and use it in all code to
> avoid mistakes?

Why?
There may be some type of partition that works fine with a 256-byte
sector size.  Using the same sector-size test for all label types would
exclude one like that.

Remember: when trying to determine what type of partition we have,
potentially all of the *_probe functions are called, so we have to
be careful not to exclude legitimate use cases.

The sector_size < 512 test above is just saying that if the sector
size is that small, then this can't be a BSD partition.