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 <kw...@redhat.com> wrote: > Am 29.01.2018 um 19:21 hat Anatol Pomozov geschrieben: >> Hi >> >> On Mon, Jan 15, 2018 at 6:49 AM, Kevin Wolf <kw...@redhat.com> 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 <anatol.pomo...@gmail.com> >> >> --- >> >> 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.
