On Thu, Aug 07, 2008 at 01:27:53AM +0200, Robert Millan wrote:
>
> Hi,
>
> It seems when adding the relocators for ELF, I failed to notice both ELF and
> a.out loaders share the same grub_multiboot_real_boot function, and hence my
> code was trashing %eax.
>
> Since this functionality is useful for the a.out loader anyway, I decided to
> implement it as well. In the process I did some cleanup, mostly to reduce
> code duplication among both users of the relocators.
>
> So here's the patch. Please comment!
I think I should ellaborate a bit on this:
> if (header->flags & MULTIBOOT_AOUT_KLUDGE)
> {
> - int ofs;
> + int offset = ((char *) header - buffer -
> + (header->header_addr - header->load_addr));
> + int load_size = ((header->load_end_addr == 0) ? file->size - offset :
> + header->load_end_addr - header->load_addr);
>
> - ofs = (char *) header - buffer -
> - (header->header_addr - header->load_addr);
> - if ((grub_aout_load (file, ofs, header->load_addr,
> - ((header->load_end_addr == 0) ? 0 :
> - header->load_end_addr - header->load_addr),
> - header->bss_end_addr))
> - !=GRUB_ERR_NONE)
> - goto fail;
> + if (header->bss_end_addr)
> + grub_multiboot_payload_size = (header->bss_end_addr -
> header->load_addr);
> + else
> + grub_multiboot_payload_size = load_size;
> + grub_multiboot_payload_dest = header->load_addr;
>
> - entry = header->entry_addr;
> + playground = grub_malloc (RELOCATOR_SIZEOF(forward) +
> grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
> + if (! playground)
> + goto fail;
> +
> + grub_multiboot_payload_orig = (long) playground +
> RELOCATOR_SIZEOF(forward);
> +
> + if ((grub_file_seek (file, offset)) == (grub_off_t) - 1)
> + goto fail;
> +
> + grub_file_read (file, grub_multiboot_payload_orig, load_size);
> + if (grub_errno)
> + goto fail;
> +
> + if (header->bss_end_addr)
> + grub_memset (grub_multiboot_payload_orig + load_size, 0,
> + header->bss_end_addr - header->load_addr - load_size);
> +
> + grub_multiboot_payload_entry_offset = header->entry_addr -
> header->load_addr;
> +
> }
The problem I had here is that grub_aout_load() was constrained to the api
used by our BSD loaders, and the relocator approach requires to intermangle
things, so I had to move it to multiboot.c and transform it.
Splitting it to a separate function (like we do for multiboot/ELF) wouldn't
be hard if this is seen as a good idea, but making it global wouldn't be so
sound IMHO: it relies on the relocator variables, so one would have to
implement relocators in each a.out loader, which doesn't sound like such a
bad idea, but the relocator code itself is biased towards multiboot (written
to preserve %eax and %ebx), and so it's not as generic as it looks, as other
load protocols might have conflicting constraints.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
