Re: [PATCH] binfmt_flat: minimum support for the Blackfin relocations

From: Bernd Schmidt
Date: Fri Sep 28 2007 - 19:49:23 EST

Andrew Morton wrote:
if (rev > OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;

`persistent' here only has meaning inside the next nesting level, so should
be moved down into that scope for readability reasons.

See below.

+ if (flat_set_persistent (relval, &persistent))
+ continue;

If this correct? flat_set_persistent() returns zero if it didn't write
anything to `persistent'. It seems strange that in the case where
flat_set_persistent() _does_ write something to `persistent', we just throw
it away by doing `continue'.

Either that, or I've misread the code and you really did mean to put
`persistent' in the outer scope, and its value is supposed to propagate
over into the next iteration of the loop. If so, that's all a bit too
tricky for it to be implemented with zero code comments, dontcha think?

The latter. We need to be able to use more data than we can fit into a single reloc, so we store a value with one reloc and reuse it with the next. There'd be no point in having this function otherwise since you could perform whatever needs to be done in flat_get_relocate_addr.

This seemed fairly obvious at the time... when you're familiar with the flat format, the loop isn't all that hard to understand. I'll add comments in the next version.

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@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at