Geert Stappers
2015-Oct-10 11:29 UTC
[syslinux] [PATCH 2/2] com32/mboot/map.c: removed trailing spaces
From: Geert Stappers <stappers at nero.gpm.stappers.nl> They were introduced by the patch for ELF64 support. --- com32/mboot/map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com32/mboot/map.c b/com32/mboot/map.c index 2e8641f..1992f14 100644 --- a/com32/mboot/map.c +++ b/com32/mboot/map.c @@ -281,7 +281,7 @@ struct multiboot_header *map_image(void *ptr, size_t len) sh[i].sh_addr = addr; } } - } else if (eh64 && !(opt.aout && mbh_len && + } else if (eh64 && !(opt.aout && mbh_len && (mbh->flags & MULTIBOOT_AOUT_KLUDGE))) { /* Load 64-bit ELF */ regs.eip = eh64->e_entry; /* Can be overridden further down... */ @@ -378,7 +378,7 @@ struct multiboot_header *map_image(void *ptr, size_t len) continue; /* SHF_ALLOC sections should have PHDRs */ align = sh64[i].sh_addralign ? sh64[i].sh_addralign : 0; - addr = map_data((char *)ptr + sh64[i].sh_offset, + addr = map_data((char *)ptr + sh64[i].sh_offset, sh64[i].sh_size, align, MAP_HIGH); if (!addr) { error("Failed to map symbol section\n"); -- 2.0.0
> From: Geert Stappers <stappers at nero.gpm.stappers.nl> > > They were introduced by the patch for ELF64 support. > ---To avoid potential confusion about what exact feature / capability is being added to mboot.c32 (see [1]), I would suggest a more accurate statement as title / subject of the patch(es): Extend Multiboot1 with support for ELF64 file format IMHO, the trivial trailing-space cleanup could be included in the same commit too, instead of adding an additional commit just for it. That is, if there is no additional correction / improvement required. Perhaps some of the TAB characters or the multiple space characters should be rather "unified" in accordance to the rest of the Syslinux code (for consistency)? (I don't know whether there is a real need to "correct" something or not.) Thank you and Best Regards, Ady. [1]: http://bugzilla.syslinux.org/show_bug.cgi?id=28
Geert Stappers
2015-Oct-13 21:17 UTC
[syslinux] com32/mboot/map.c: removed trailing spaces
On Sat, Oct 10, 2015 at 03:10:26PM +0300, Ady via Syslinux wrote:> From: Geert Stappers <stappers at nero.gpm.stappers.nl> > > > > com32/mboot/map.c: removed trailing spaces > > > > They were introduced by the patch for ELF64 support. > > > IMHO, the trivial trailing-space cleanup could be included in the same > commit too, instead of adding an additional commit just for it. That > is, if there is no additional correction / improvement required.I think it is a good thing to just accept a patch that is good enough and do reflow work in an additional patch. I see several advantages: * a clear signal of "patch accepted" * making visible how a patch is made better * no merge conflict for those who have the original patch applied Groeten Geert Stappers -- Leven en laten leven