On Sun, Jan 02, 2011 at 01:54:28AM +0100, Josip Rodin wrote:
> 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 :)
Good points!
I will aredo this to use increment-by-four and document this in v3 of the patchset.
Sam
--
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
