Messages in this thread | | | Date | Thu, 20 Sep 2007 14:04:26 +0200 | From | Bernd Schmidt <> | Subject | Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations |
| |
Paul Mundt wrote: > I find it a bit disconcerting that blackfin already depends on this > in-tree without there being any earlier discussion on making these > changes.
Parts of the initial submission were picked up (the include/asm directory), other's weren't. Little we can do about that.
>>>> */ >>>> if (rev > OLD_FLAT_VERSION) { >>>> + unsigned long persistent = 0; >>>> for (i=0; i < relocs; i++) { >>>> unsigned long addr, relval; >>>> >>>> @@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm, >>>> relocated (of course, the address has to be >>>> relocated first). */ >>>> relval = ntohl(reloc[i]); >>>> + if (flat_set_persistent (relval, &persistent)) >>>> + continue; >>>> addr = flat_get_relocate_addr(relval); >>>> rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1); >>>> if (rp == (unsigned long *)RELOC_FAILED) { > > I don't much care for this API. It's shuffling around a temporary > variable for the architecture code that's set for certain relocations > that are otherwise unhandled. > > Since all the architecture is interested in is the relval that has > associated "persistent" data encoded in it, why don't we just have a stub > to give the architecture a chance to validate the relval before the > flat_get_relocate_addr() and move this stuff there instead? ie, blackfin > takes this out-of-line and manages its persistent value there.
What is flat_set_persistent other than a stub to validate the relval? I'm not at all sure what you're proposing or how it would be different.
> load_flat_file() is ugly enough without dumping more architecture > callback abuses in it.
The other maintainers who have spoken up didn't seem to think this was ugly, or an abuse. I'm surprised to hear language like that when discussing a patch that adds an if statement, a local variable and one parameter in a function call.
Bernd -- This footer brought to you by insane German lawmakers. Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |