"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
>> +struct diskType disk_types[] = {
>
> This can be const const - surprised Jim didn't catch this :-)
> +struct diskType disk_types[] = {
Had to leave some for you. :-P
Now that you mention it, though, that should be "static", too.
>> +enum partTableType {
>> + VIR_STORAGE_POOL_DISK_DOS = 1,
>> + VIR_STORAGE_POOL_DISK_DVH,
>> + VIR_STORAGE_POOL_DISK_GPT,
>> + VIR_STORAGE_POOL_DISK_MAC,
>> + VIR_STORAGE_POOL_DISK_BSD,
>> + VIR_STORAGE_POOL_DISK_PC98,
>> + VIR_STORAGE_POOL_DISK_SUN,
>> + VIR_STORAGE_POOL_DISK_LVM2,
>> + /* the "unknown" disk is 1 billion (and not, for instance, -1), to make
>> + sure it doesn't run afoul of error checking */
>> + VIR_STORAGE_POOL_DISK_UNKNOWN = 1 * 1024 * 1024 * 1024,
>
> I don't understand why it has to be 1-billion ? For the enum support to
> work correctly, we need this to be contiguous, starting from zero, and
> and as the last element have
>
> VIR_STORAGE_POOL_DISK_LAST
>
> This perhaps suggests that DISK_UNKNOWN should be the first entry so
> that if you have a zero'd struct with a disk type, it'll get defaulted
> to UNKOWN
Or maybe leave it as = 1, to force explicit definition to an enum value.
Then, wherever it's zeroed but not explicitly defined
it will show up as undefined (possibly triggering some sort
of error), rather than proceeding with a mere UNKNOWN.
I guess it depends on the relative worth of being able to
initialize with calloc-style allocation vs. cost of explicit
definition everywhere.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list