Subject: Re: Cardbus mapreg bug
To: Martin Husemann <martin@duskware.de>
From: Rhialto <rhialto@falu.nl>
List: tech-kern
Date: 04/21/2006 17:46:59
On Fri 21 Apr 2006 at 10:53:21 +0200, Martin Husemann wrote:
> Could someone borrow me a pair of eyes?
> 
> The following patch seems neded to avoid uninitialized use of "handle"
> later in the code. But unfortunately sc->sc_ioh/sc->sc_memh do not exists
> (so this does not even compile).
> 
> Remember that bus_space_handle_t is an opaque type and we have no idea
> how to initialize it (it is a struct on some archs).
> 
> How is this supposed to work?

I know nothing about cardbus, I'm just looking at the file.

I see that handle is first mentioned in one of two ways, depending on the
rbus define:

    if (status == 0) {
#if rbus
	if ((*cf->cardbus_space_alloc)(cc, rbustag, base, size, mask,
	    size, busflags | flags, &base, &handle)) {
#else
	if (bus_space_alloc(bustag, start, end, size, size, 0, 0, &base, &handle)) {
#endif
    }

The manpage for bus_space_alloc claims it initialises handle (via
*&handle). So here is where it would normally gets its value. I'll
assume that the indirect call (cardbus_space_alloc) works in the same
way.

It is then (maybe) used to copy the obtained value to *handlep (if not
a NULL pointer).

However the initialisation only happens if status == 0. I'd say that the
later assignment should be made to depend on the same condition. If the
compiler doesn't understand it it safe then, it looks like it can be
moved into the same if() block, that would make it obvious.

It looks like "status" is a deceptive name, if cardbus_mem_find() does
what I expect it does, "found" would be better.

"base" may suffer from the same issue, except it is referenced earlier
and might be initialised then.

I think I now see what this function is supposed to do: either allocate
new bus space, or if it had been allocated before, return the earlier
instance.

Which leads me to this solution: if cardbus_mem_find() manages to dig up
an earlier "base" value, it might as well return the corresponding
"handle" value as well.

Can that be right?

> Martin
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert      -- You author it, and I'll reader it.
\X/ rhialto/at/xs4all.nl        -- Cetero censeo "authored" delendum esse.