Re: [Xen-devel] [PATCH v3 06/25] libelf-loader: introduce elf_load_image

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.01.12 at 19:13, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Thu, 15 Dec 2011, Jan Beulich wrote:
>> >>> On 15.12.11 at 17:30, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> > 
>> > Implement a new function, called elf_load_image, to perform the actually
>> > copy of the elf image and clearing the padding.
>> > The function is implemented as memcpy and memset when the library is
>> > built as part of the tools, but it is implemented as raw_copy_to_guest and
>> > raw_clear_guest when built as part of Xen, so that it can be safely called
>> > with an HVM style dom0.
>> > 
>> > 
>> > Changes in v3:
>> > 
>> > - switch to raw_copy_to_guest and raw_clear_guest.
>> > 
>> > 
>> > Changes in v2:
>> > 
>> > - remove CONFIG_KERNEL_NO_RELOC.
>> > 
>> > 
>> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> > Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
>> > ---
>> >  xen/common/libelf/libelf-loader.c |   17 +++++++++++++++--
>> >  1 files changed, 15 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/xen/common/libelf/libelf-loader.c 
>> > b/xen/common/libelf/libelf-loader.c
>> > index 1ccf7d3..d2cb5b3 100644
>> > --- a/xen/common/libelf/libelf-loader.c
>> > +++ b/xen/common/libelf/libelf-loader.c
>> > @@ -107,11 +107,25 @@ void elf_set_log(struct elf_binary *elf, 
>> > elf_log_callback *log_callback,
>> >      elf->log_caller_data = log_caller_data;
>> >      elf->verbose = verbose;
>> >  }
>> > +
>> > +static void elf_load_image(void *dst, const void *src, uint64_t filesz, 
>> > uint64_t memsz)
>> > +{
>> > +    memcpy(dst, src, filesz);
>> > +    memset(dst + filesz, 0, memsz - filesz);
>> > +}
>> >  #else
>> > +#include <asm/guest_access.h>
>> > +
>> >  void elf_set_verbose(struct elf_binary *elf)
>> >  {
>> >      elf->verbose = 1;
>> >  }
>> > +
>> > +static void elf_load_image(void *dst, const void *src, uint64_t filesz, 
>> > uint64_t memsz)
>> > +{
>> > +    raw_copy_to_guest(dst, src, filesz);
>> > +    raw_clear_guest(dst + filesz, memsz - filesz);
>> 
>> I thought I had mentioned this already - calling user/guest access
>> functions without checking their return value is bogus.
> 
> I am going to add two checks here and print a message in case of errors,
> however I am not going to propagate the error up, considering that there
> is no error checking in elf_load_binary and it doesn't return an error.
> Would that be OK for you?

Actually, I would prefer making the entire call chain pass up the error,
and handle it at the top-most caller. Unless an error is impossible here
(i.e. the functions are being used solely to get the necessary HVM
semantics taken care of, in which case casting their results to void
and annotating them with a respective comment would seem sufficient
to me).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel