[Qemu-devel] Re: [PATCH 2/2] multiboot: Limit number of multiboot modules

[Kevin Wolf]

Am 21.10.2009 01:05, schrieb Juan Quintela:
> Adam Lackorzynski <address@hidden> wrote:
> 
> Hi
> 
>> +enum {
>> +    /* Multiboot info */
>> +    MBI_FLAGS       = 0,
>> +    MBI_MEM_LOWER   = 4,
>> +    MBI_MEM_UPPER   = 8,
>> +    MBI_BOOT_DEVICE = 12,
>> +    MBI_CMDLINE     = 16,
>> +    MBI_MODS_COUNT  = 20,
>> +    MBI_MODS_ADDR   = 24,
>> +    MBI_MMAP_ADDR   = 48,
>> +
>> +    MBI_SIZE        = 88,
>> +
>> +    /* Multiboot modules */
>> +    MB_MOD_START    = 0,
>> +    MB_MOD_END      = 4,
>> +    MB_MOD_CMDLINE  = 8,
>> +
>> +    MB_MOD_SIZE     = 16,
>> +
>> +    /* Region offsets */
>> +    ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
>> +    ADDR_MBI      = ADDR_E820_MAP + 0x500,
>> +
>> +    /* Multiboot flags */
>> +    MULTIBOOT_FLAGS_MEMORY      = 1 << 0,
>> +    MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
>> +    MULTIBOOT_FLAGS_CMDLINE     = 1 << 2,
>> +    MULTIBOOT_FLAGS_MODULES     = 1 << 3,
>> +    MULTIBOOT_FLAGS_MMAP        = 1 << 6,
>> +};
> 
> Why do you use a single enum, without name, and repeating the values?
> I think that using more than one enum is better here.

Right, this looks a bit strange even though it's correct. Otherwise the
new patch looks okay and I'd suggest to submit it for inclusion.

Kevin