Re: [PATCH 6/7] mkimage: support images which require full relocation at mkimage time.

[Ian Campbell]

On Sun, 2013-12-29 at 22:53 +0000, Leif Lindholm wrote:
> > diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> > index a2bd4c1..267beaa 100644
> > --- a/util/grub-mkimage.c
> > +++ b/util/grub-mkimage.c
> > @@ -65,6 +65,7 @@ static struct argp_option options[] = {
> >     /* TRANSLATORS: platform here isn't identifier. It can be translated.  
> > */
> >     N_("use images and modules under DIR [default=%s/<platform>]"), 0},
> >    {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 
> > 0},
> > +  {"target-address", 'T', N_("ADDR"), 0, N_("set kernel target address 
> > [default=%d]"), 0},
> 
> For this to actually print a default value, you need to also update
> the help_filter function. Also, you probably want a %p here.

I hadn't noticed that, but given the lack of any default in the above it
now seems obvious. Thanks.

[...]
> (Of course, you probably want to abstract away the ARM_UBOOT bit...)

That might be tricky in practice -- what do you do if the user hasn't
given any target yet?

I'm more than a little inclined to suggest that this ought to say
(literally) "[default=PLATFORM-SPECIFIC]" or something non-committal. A
lot simpler with no odd corner cases.

[...] 
> > -      *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> > -                                            section_vaddresses, sections,
> > -                                            section_entsize, num_sections,
> > -                                            strtab, ia64jmp_off
> > -                                            + image_target->vaddr_offset,
> > -                                            2 * ia64jmpnum + (got / 8),
> > -                                            image_target);
> > +      if (!grub_image_needs_abs_reloc(image_target))
> > +     *reloc_size = SUFFIX (make_reloc_section) (e, reloc_section,
> > +                                                section_vaddresses, 
> > sections,
> > +                                                section_entsize, 
> > num_sections,
> > +                                                strtab, ia64jmp_off
> > +                                                + 
> > image_target->vaddr_offset,
> > +                                                2 * ia64jmpnum + (got / 8),
> > +                                                image_target);
> 
> The above is only indentation changes?

Other than the addition of the if conditional, the rest is just the
indentation changes arising from that addition.

Ian.