Re: [bug] ata subsystem related crash with latest -git

Linus Torvalds
2007-10-18T01:08:41Z

On Thu, 18 Oct 2007, FUJITA Tomonori wrote:
>
> Looks that (sglist) - 1 isn't initialized and we use sg_next for it?

sg_next() - as it stands now - never actually looks at the SG that its
argument points to: it explicitly *only* looks at the next one.

That's the bug. If sg_next() looked at the actual *current* sg entry, we
wouldn't have any issues to begin with, and that's what I'm arguing we
should do in the longer run (where "longer run" is defined as "when Jens
does it asap").

So the hacky solution as it stands right now is to actually use the
behaviour of "sg_next()" to our advantage in for_each_sg(), and starting
off by setting sg to (sglist)-1. That way we can do the "sg_next()" (which
will *not* look at the uninitialized space before the array) before
entering the loop, regardless of whether it's the first time through the
loop or not.

So no, it's not technically "strictly conforming ANSI C", because we use a
pointer (not do not dereference it!) outside of its allocation, which is
strictly speaking against the standard. However, the kernel does those
kinds of things all the time, since the kernel does its own memory
allocation, so that's not actually relevant.

The *standard* may say that you cannot decrement a pointer to before the
beginning of the object and then increment it back up again and be
strictly conforming, but the fact is, we very much depend on contiguous
and flat kernel pointer model, and always have (and probably always will)

Linus