Re: New FBDev patch

[Russell King]

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