Re: New FBDev patch

[Russell King <rmk+lkml@arm.linux.org.uk>]

On Fri, Jan 09, 2004 at 07:54:50PM +0000, James Simmons wrote:
> > sizeof(u32) != 32.  Proper fix is to place the pseudopalette array
> > inside cfb_info, and dispense with this addition here.
> 
> You have to make sure the struct fb_info pseudopalette points to the data 
> in cfb_info. Actually only drivers should allocate the pseudopalette at 
> boot time if the hardware doesn't support mode change. In the other case 
> the pseudopalette should be allocated in set_par. 

I respectfully disagree.  It is not possible to error out of the set_par()
function - for instance, fb_set_var() contains this code:

                        if (info->fbops->fb_set_par)
                                info->fbops->fb_set_par(info);

There isn't any error return checking (so why does fb_set_par return an
int when it isn't used?  It's fairly misleading.)

This all means that if the allocation of the pseudo_palette in set_par
fails, there's no way to abort the mode change - you _will_ oops in the
other fbcon layers due to a NULL pseudo_palette pointer.

Plus, we're only talking about an array of 16 32-bit words or 64 bytes.
That's hardly worth the extra code complexity to separately dynamically
allocate.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core


-------------------------------------------------------
This SF.net email is sponsored by: Perforce Software.
Perforce is the Fast Software Configuration Management System offering
advanced branching capabilities and atomic changes on 50+ platforms.
Free Eval! http://www.perforce.com/perforce/loadprog.html