Re: [PATCH parted 1/5] libparted: add ped_device_get_xxx_aligment() functions

[Jim Meyering]

Hans de Goede wrote:
> Note, these patches may apply to master with a couple of lines offset
> left and right, as I also have some patches in my local tree to update
> parted to keep working with upcoming lvm2 (libdevmapper) changes, I'm still
> waiting on testing feedback for these (the libdevmapper) patches before
> submitting them.
>
> Regards,
>
> Hans
>
> p.s.
>
> I also plan to backport this set of 5 to libparted-1.9 and add them to the
> Fedora packages for F-13.
>
> On 10/28/2009 10:20 PM, Hans de Goede wrote:
>> Add ped_device_get_minimal_aligment() and ped_device_get_optimal_aligment()
>> functions to libparted.
>> ---
>>   include/parted/device.h  |    8 ++++++++
>>   include/parted/natmath.h |    1 +
>>   libparted/device.c       |   18 ++++++++++++++++++
>>   3 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/parted/device.h b/include/parted/device.h
>> index 151305f..31aec86 100644
>> --- a/include/parted/device.h
>> +++ b/include/parted/device.h
>> @@ -92,6 +92,8 @@ struct _PedDevice {
>>           void*           arch_specific;
>>   };
>>
>> +#include<parted/natmath.h>
>> +
>>   /**
>>    * List of functions implementing architecture-specific operations.
>>    */
>> @@ -112,6 +114,9 @@ struct _PedDeviceArchOps {
>>           PedSector (*check) (PedDevice* dev, void* buffer,
>>                               PedSector start, PedSector count);
>>           void (*probe_all) ();
>> +        /* These functions are optional */
>> +        PedAlignment* (*get_minimal_aligment)(PedDevice* dev);
>> +        PedAlignment* (*get_optimal_aligment)(PedDevice* dev);

Please add "const" and adjust spacing: space before "*", not after:

            PedAlignment *(*get_minimal_aligment)(const PedDevice *dev);
            PedAlignment *(*get_optimal_aligment)(const PedDevice *dev);

The const is required, when the pointed to object is not modified,
and especially important on a public interface.
Otherwise, a caller with a const *dev would have to cast away const
in order to avoid compiler warnings.  And we all want to avoid casts.

>>   };
>>
>>   #include<parted/constraint.h>
>> @@ -141,6 +146,9 @@ extern PedSector ped_device_check (PedDevice* dev, void* 
>> buffer,
>>                                      PedSector start, PedSector count);
>>   extern PedConstraint* ped_device_get_constraint (PedDevice* dev);
>>
>> +extern PedAlignment* ped_device_get_minimal_aligment(PedDevice* dev);
>> +extern PedAlignment* ped_device_get_optimal_aligment(PedDevice* dev);
>> +
>>   /* private stuff ;-) */
>>
>>   extern void _ped_device_probe (const char* path);
>> diff --git a/include/parted/natmath.h b/include/parted/natmath.h
>> index cd7679d..eaa84d1 100644
>> --- a/include/parted/natmath.h
>> +++ b/include/parted/natmath.h
>> @@ -31,6 +31,7 @@ typedef struct _PedAlignment       PedAlignment;
>>
>>   #include<parted/disk.h>
>>   #include<parted/device.h>
>> +#include<parted/geom.h>

Might as well indent consistently.

>>
>>   #define PED_MIN(a, b)      ( ((a)<(b)) ? (a) : (b) )
>>   #define PED_MAX(a, b)      ( ((a)>(b)) ? (a) : (b) )
>> diff --git a/libparted/device.c b/libparted/device.c
>> index 294fec4..6a41512 100644
>> --- a/libparted/device.c
>> +++ b/libparted/device.c
>> @@ -444,5 +444,23 @@ ped_device_get_constraint (PedDevice* dev)
>>           return c;
>>   }
>>
>> +PedAlignment*
>> +ped_device_get_minimal_aligment(PedDevice* dev)
>> +{
>> +        if (ped_architecture->dev_ops->get_minimal_aligment)
>> +                return ped_architecture->dev_ops->get_minimal_aligment(dev);
>> +
>> +        return NULL; /* ped_alignment_none */
>> +}
>> +
>> +PedAlignment*
>> +ped_device_get_optimal_aligment(PedDevice* dev)
>> +{
>> +        if (ped_architecture->dev_ops->get_optimal_aligment)
>> +                return ped_architecture->dev_ops->get_optimal_aligment(dev);
>> +
>> +        return NULL; /* ped_alignment_none */
>> +}
>> +
>>   /** @} */
>>