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
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/