Re: [PATCH 16/18] cdev: use cdev::dos_partition_type only if cdev_is_mbr_partitioned

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 31.05.23 20:54, Marco Felsch wrote:
> On 23-05-31, Ahmad Fatoum wrote:
>> dos_partition_type == 0 can mean that either a partition is not
>> a MBR partition or that it indeed has a partition type of 0x00.
>>
>> In preparation for using that field in a union, explicitly check if we
>> have a MBR partition.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/blspec.c | 2 +-
>>  fs/fs.c         | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/blspec.c b/common/blspec.c
>> index e95a8dba8d76..8c7970da8915 100644
>> --- a/common/blspec.c
>> +++ b/common/blspec.c
>> @@ -729,7 +729,7 @@ int blspec_scan_device(struct bootentries *bootentries, struct device *dev)
>>  		 * partition with the MBR type id of 0xEA already exists it
>>  		 * should be used as $BOOT
>>  		 */
>> -		if (cdev->dos_partition_type == 0xea) {
>> +		if (cdev_is_mbr_partitioned(cdev->master) && cdev->dos_partition_type == 0xea) {
> 
> Since you already various helpers to drop priv direct access, what
> about:
> 
> 		if (cdev_dos_partition_type(cdev) == 0xea)
> 
> Within the helper you can check for the cdev_is_mbr_partitioned().

We only have a single call site and there'll be a separate series that adds:

if (cdev_is_gpt_partitioned(cdev->master) && guid_equal(&partcdev->typeuuid, blspec_xbootldr_guid))
	/* use that */;

after this line. So I'd rather leave it as is.

Thanks,
Ahmad

> 
> ?
> 
> Regards,
>   Marco
> 
>>  			ret = blspec_scan_cdev(bootentries, cdev);
>>  			if (ret == 0)
>>  				ret = -ENOENT;
>> diff --git a/fs/fs.c b/fs/fs.c
>> index 2d2d327c5fbc..9a92e6e251e5 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -108,7 +108,7 @@ void cdev_print(const struct cdev *cdev)
>>  
>>  	if (cdev->filetype)
>>  		nbytes += printf("Filetype: %s\t", file_type_to_string(cdev->filetype));
>> -	if (cdev->dos_partition_type)
>> +	if (cdev_is_mbr_partitioned(cdev->master))
>>  		nbytes += printf("DOS parttype: 0x%02x\t", cdev->dos_partition_type);
>>  	if (*cdev->partuuid || *cdev->diskuuid)
>>  		nbytes += printf("%sUUID: %s", cdev_is_partition(cdev) ? "PART" : "DISK",
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |