On Sun, Jan 02, 2011 at 12:48:35AM +0100, Sam Ravnborg wrote:
> On Sun, Jan 02, 2011 at 12:29:17AM +0100, Josip Rodin wrote:
> > On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> > > + for (i = 0; i < LOOKBACK; ++i) {
> > > + if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
> > > + buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
> > > + return offset + i;
> > > + }
> > > + }
> >
> > > - for (q = buffer, r = q + 512; q < r; q += 4) {
> > > - if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
> > > - break;
> > > - }
> >
> > BTW, while the new code is eminently more readable in general, is there
> > a reason to slow down that search by using increments of 1 instead of 4?
> > Did the old one ever miss?
>
> I used increment by 1 because there was nothing in head_32/_64 comments
> that guaranteed that "HdrS" would be on a 4-byte aligned address.
> [The actual implmentation guarantees this today].
>
> I had two options:
> - use increment by one
> - use increment by 4 and explain why it was safe to do so
>
> And I just picked the simpler variant.
>
> So I do not fix any know issue - just playing safe.
> And speed does not matter here....
OK, just thought you might want to document that, given all the discussion
about arcane code. (Thanks.) I would actually hesitate from calling this one
the simpler variant, because making as few changes as possible compared to
the old code usually makes a refactoring simpler to debug if the need arises :)
--
2. That which causes joy or happiness.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
