Re: [4KSTACK][2.6.6] Stack overflow in radeonfb

From: Jörn Engel
Date: Fri May 14 2004 - 05:02:24 EST


On Fri, 14 May 2004 08:56:21 +1000, Benjamin Herrenschmidt wrote:
> >
> > I'm not sure what the point behind the radeon_write_mode() is at all.
> > The best solution could be to just merge radeon_write_mode() and
> > radeonfb_set_par() into a single function and do the tons of OUTREG()
> > directly. In that case, don't bother to fix any typos
>
> No, they should stay separate functions. I may use write_mode in a
> different way in the future (like restoring previous mode on module
> unload for example) and I'm very much against merging 2 already too big
> function into one huge horror.

Not sure if the combined function would really be bigger than either
one alone. Basically, set_par writes to a temp struct and write_mode
writes from the temp struct to hardware. Sounds like quite a bit of
redundant code could be removed.

With more users for write_mode the seperate function makes sense
again, so you should keep it. Just the second argument isn't valid
imo.

Jörn

--
Correctness comes second.
Features come third.
Performance comes last.
Maintainability is needed for all of them.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/