On 2018-01-15 07:54, Kevin Wolf wrote:
If need be, would you be willing to accept updated versions of these
patches (with another review, of course) without the test file? I will
deliver the test file later once I get company approvals. I don't want
the test file to continue holding everything up in the meantime.
Am 21.12.2017 um 18:25 hat Jack Schwartz geschrieben:
Properly account for the possibility of multiboot kernels with a zero
bss_end_addr. The Multiboot Specification, section 3.1.3 allows for
kernels without a bss section, by allowing a zeroed bss_end_addr multiboot
Do some cleanup to multiboot.c as well:
- Remove some unused variables.
- Use more intuitive header names when displaying fields in messages.
- Change fprintf(stderr...) to error_report
There are some conflicts with Anatol's (CCed) multiboot series:
None if these should be hard to resolve, but it would be good if you
could agree with each other whose patch series should come first, and
then the other one should be rebased on top of that.
1) Ran the "make check" test suite.
2) Booted multiboot kernel with bss_end_addr=0. (I rolled my own
grub multiboot.elf test "kernel" by modifying source.) Verified
with gdb that new code that reads addresses/offsets from multiboot
header was accessed.
3) Booted multiboot kernel with non-zero bss_end_addr.
4) Uncommented DEBUG_MULTIBOOT in multiboot.c and verified messages worked.
5) Code has soaked in an internal repo for two months.
Can you integrate your test kernel from 2) in tests/multiboot/ so we can
keep this as a regression test?
Patchwork links, for reference:
1/4 multiboot: bss_end_addr can be zero
2/4 multiboot: Remove unused variables from multiboot.c
3/4 multiboot: Use header names when displaying fields
4/4 multiboot: fprintf(stderr...) -> error_report()
Jack Schwartz (4):
multiboot: bss_end_addr can be zero
multiboot: Remove unused variables from multiboot.c
multiboot: Use header names when displaying fields
multiboot: fprintf(stderr...) -> error_report()
Apart from the conflicts, the patches look good to me.