qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array


From: Anatol Pomozov
Subject: Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct
Date: Fri, 9 Feb 2018 13:48:02 -0800

Hi Kevin

Is the patch series look good? Are there any other unresolved issues?

On Mon, Jan 29, 2018 at 12:02 PM, Kevin Wolf <address@hidden> wrote:
> Am 29.01.2018 um 19:21 hat Anatol Pomozov geschrieben:
>> Hi
>>
>> On Mon, Jan 15, 2018 at 6:49 AM, Kevin Wolf <address@hidden> wrote:
>> > Am 13.10.2017 um 01:54 hat Anatol Pomozov geschrieben:
>> >> Using C structs makes the code more readable and prevents type conversion
>> >> errors.
>> >>
>> >> Borrow multiboot1 header from GRUB project.
>> >>
>> >> Signed-off-by: Anatol Pomozov <address@hidden>
>> >> ---
>> >>  hw/i386/multiboot.c         | 124 +++++++++------------
>> >>  hw/i386/multiboot_header.h  | 254 
>> >> ++++++++++++++++++++++++++++++++++++++++++++
>> >>  tests/multiboot/mmap.c      |  14 +--
>> >>  tests/multiboot/mmap.out    |  10 ++
>> >>  tests/multiboot/modules.c   |  12 ++-
>> >>  tests/multiboot/modules.out |  10 ++
>> >>  tests/multiboot/multiboot.h |  66 ------------
>> >>  7 files changed, 339 insertions(+), 151 deletions(-)
>> >>  create mode 100644 hw/i386/multiboot_header.h
>> >>  delete mode 100644 tests/multiboot/multiboot.h
>> >
>> > This is a good change in general. However, I'm not sure if we should
>> > really replace the header used in the tests. After all, the tests also
>> > test whether things are at the right place, and if there is an error in
>> > the header file, we can only catch it if the tests keep using their own
>> > definitions.
>>
>> The added multibooh.h is from GRUB - the developers of multiboot. I
>> think we can trust it. Having a single header will make future tests
>> maintainence easier.
>>
>> But if you feel strongly that qemu tests should use it's own version
>> of multiboot header then I can add it back.
>
> No, I don't feel strongly, just wanted to mention that there is an
> advantage of having a separate header in case you had not thought of it.
> The decision is up to you.

I think it is better to use one standard trusted header. This way we
reduce maintenance cost.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]