[lkml]   [2007]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] binfmt_flat: minimum support for the Blackfin relocations
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
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2007-09-29 01:51    [W:0.068 / U:2.220 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site