Re: [PATCH] FDPIC: add hook for arches to customize program header parsing

[Mike Frysinger <vapier.adi@gmail.com>]

On Tue, Jun 23, 2009 at 01:32, Andrew Morton wrote:
> On Tue, 16 Jun 2009 04:24:48 -0400 Mike Frysinger wrote:
>> From: Jie Zhang <jie.zhang@analog.com>
>> The Blackfin port has custom program header flags/addresses for
>> automatically loading regions into the dedicated on-chip SRAM.  So add a
>> hook for ports to leverage.
>>
>> Signed-off-by: Jie Zhang <jie.zhang@analog.com>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> CC: Bernd Schmidt <bernds_cb1@t-online.de>
>> Acked-by: David Howells <dhowells@redhat.com>
>> Acked-by: Paul Mundt <lethal@linux-sh.org>
>> Acked-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>> Andrew: could you pick this up ?  same patch as before, just with the
>> additional tags added.
>
> I suppose we want to squeeze this into 2.6.31.

would be nice, but not terribly critical

>> --- a/fs/binfmt_elf_fdpic.c
>> +++ b/fs/binfmt_elf_fdpic.c
>> @@ -1105,6 +1105,13 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
>>                   ELF_FDPIC_FLAG_CONTIGUOUS)
>>                       load_addr += PAGE_ALIGN(phdr->p_memsz + disp);
>>
>> +#ifndef ELF_FDPIC_PLAT_PROCESS_PHDR
>> +# define ELF_FDPIC_PLAT_PROCESS_PHDR(mm, params, phdr, maddr, disp) 0
>> +#endif
>> +             ret = ELF_FDPIC_PLAT_PROCESS_PHDR(mm, params, phdr, &maddr, &disp);
>> +             if (ret)
>> +                     return ret;
>> +
>
> Wouldn't it be simpler to do
>
> #ifdef ELF_FDPIC_PLAT_PROCESS_PHDR
>                ret = ELF_FDPIC_PLAT_PROCESS_PHDR(mm, params, phdr, &maddr,
>                                                  &disp);
>                if (ret)
>                        return ret;
> #endif
>
> ?
>
> If there's any prospect that we'll use ELF_FDPIC_PLAT_PROCESS_PHDR() a
> second time from within this file then yes, adding a definition as
> you did is appropriate.  But it should be near the top of the file, not
> stuck stupidly in the middle of a C function.

the purpose was to stick to current conventions used in the file and
to maximize the amount of code compiled.  i dont particularly care wrt
the method used here.
-mike