kandagatla.Srinivas wrote:
> Hi All,
>
> There is a major Bug in DirectFB-0.9.25.1which will crash DFB.
Thanks for spotting this.
> Look at the below code..from(system/fbdev/fbdev.c)
>
> shared->cmap_memory = SHMALLOC( pool_data, 256 * 2 * 12 );
> shared->orig_cmap.start = 0;
> shared->orig_cmap.len = 256;
> shared->orig_cmap.red = shared->cmap_memory + 256 * 2 * 0;
> shared->orig_cmap.green = shared->cmap_memory + 256 * 2 * 1;
> shared->orig_cmap.blue = shared->cmap_memory + 256 * 2 * 2;
> shared->orig_cmap.transp = shared->cmap_memory + 256 * 2 * 3;
>
> if (ioctl( dfb_fbdev->fd, FBIOGETCMAP, &shared->orig_cmap ) < 0) {
> D_PERROR( "DirectFB/FBDev: "
> "Could not retrieve palette for backup!\n" );
> SHFREE( pool_data, shared->cmap_memory );
The easiest fix would be to simply not free the memory here,
but saving 2k is a good idea :)
> The below patch fixes the above discussed problem.
> Mail me if you want more detail..
>
> diff -urN 1/systems/fbdev/fbdev.c 2/systems/fbdev/fbdev.c
> --- 1/systems/fbdev/fbdev.c 2006-06-29 10:09:34.000000000 +0530
> +++ 2/systems/fbdev/fbdev.c 2006-05-03 12:52:37.000000000 +0530
> @@ -508,36 +508,33 @@
> dfb_fbdev_var_to_mode( &shared->current_var,
> &shared->current_mode );
>
> -/* shared->cmap_memory = SHMALLOC( pool_data, 256 * 2 * 12 );*/
> - shared->orig_cmap_memory = SHMALLOC( pool_data, 256 * 2 * 4 );
> + shared->cmap_memory = SHMALLOC( pool_data, 256 * 2 * 12 );
Took a few seconds to recognize that your patch is reversed :)
> shared->orig_cmap.start = 0;
> shared->orig_cmap.len = 256;
> - shared->orig_cmap.red = shared->orig_cmap_memory + 256 * 2 * 0;
> - shared->orig_cmap.green = shared->orig_cmap_memory + 256 * 2 * 1;
> - shared->orig_cmap.blue = shared->orig_cmap_memory + 256 * 2 * 2;
> - shared->orig_cmap.transp = shared->orig_cmap_memory + 256 * 2 * 3;
> + shared->orig_cmap.red = shared->cmap_memory + 256 * 2 * 0;
> + shared->orig_cmap.green = shared->cmap_memory + 256 * 2 * 1;
> + shared->orig_cmap.blue = shared->cmap_memory + 256 * 2 * 2;
> + shared->orig_cmap.transp = shared->cmap_memory + 256 * 2 * 3;
>
> if (ioctl( dfb_fbdev->fd, FBIOGETCMAP, &shared->orig_cmap ) < 0) {
> D_PERROR( "DirectFB/FBDev: "
> "Could not retrieve palette for backup!\n" );
> - SHFREE( pool_data, shared->orig_cmap_memory );
> + SHFREE( pool_data, shared->cmap_memory );
> shared->orig_cmap.len = 0;
Maybe it's even better to memset the cmap struct with zero to also NULL
all the pointers and protect against accidental usage later on.
> }
>
> - shared->temp_cmap_memory = SHMALLOC( pool_data, 256 * 2 * 4 );
> shared->temp_cmap.len = 256;
> - shared->temp_cmap.red = shared->temp_cmap_memory + 256 * 2 * 4;
> - shared->temp_cmap.green = shared->temp_cmap_memory + 256 * 2 * 5;
> - shared->temp_cmap.blue = shared->temp_cmap_memory + 256 * 2 * 6;
> - shared->temp_cmap.transp = shared->temp_cmap_memory + 256 * 2 * 7;
> + shared->temp_cmap.red = shared->cmap_memory + 256 * 2 * 4;
> + shared->temp_cmap.green = shared->cmap_memory + 256 * 2 * 5;
> + shared->temp_cmap.blue = shared->cmap_memory + 256 * 2 * 6;
> + shared->temp_cmap.transp = shared->cmap_memory + 256 * 2 * 7;
You forgot to change the offsets from 4,5,6,7 to 0,1,2,3 :)
> - shared->current_cmap_memory = SHMALLOC( pool_data, 256 * 2 * 4 );
> shared->current_cmap.len = 256;
> - shared->current_cmap.red = shared->current_cmap_memory + 256 *
> 2 * 8;
> - shared->current_cmap.green = shared->current_cmap_memory + 256 *
> 2 * 9;
> - shared->current_cmap.blue = shared->current_cmap_memory + 256 *
> 2 * 10;
> - shared->current_cmap.transp = shared->current_cmap_memory + 256 *
> 2 * 11;
> + shared->current_cmap.red = shared->cmap_memory + 256 * 2 * 8;
> + shared->current_cmap.green = shared->cmap_memory + 256 * 2 * 9;
> + shared->current_cmap.blue = shared->cmap_memory + 256 * 2 * 10;
> + shared->current_cmap.transp = shared->cmap_memory + 256 * 2 * 11;
dito
> dfb_fbdev_get_pci_info( shared );
>
> @@ -667,12 +664,7 @@
> "Could not restore palette!\n" );
> }
>
> - if( shared->orig_cmap.len> 0) /* Check if memory IS allocated*/
> - SHFREE( shared->shmpool_data, shared->orig_cmap_memory );
> - if( shared->current_cmap.len> 0)/* Check if memory IS allocated*/
> - SHFREE( shared->shmpool_data, shared->current_cmap_memory );
> - if( shared->temp_cmap.len> 0)/* Check if memory IS allocated*/
> - SHFREE( shared->shmpool_data, shared->orig_cmap_memory );
Better keep pointers NULL if memory is not allocated and check for that
only.
By that opportunity, we should add checks after SHMALLOC() to do
D_OOSHM() if it returns NULL :-)
--
Best regards,
Denis Oliver Kropp
.------------------------------------------.
| DirectFB - Hardware accelerated graphics |
| http://www.directfb.org/ |
"------------------------------------------"
_______________________________________________
directfb-dev mailing list
[email protected]
http://mail.directfb.org/cgi-bin/mailman/listinfo/directfb-dev
