> Passing NULL for pbits into fbScreenInit is a treacherous
> thing to do. It works, because we rapidly resize
> the screen, and because the normal path never uses
> the screen pixmap.
>
> However, having NULL for pbits has side effects that derive
> from the fact that NULL prevents ModifyPixmapHeader
> from doing anything, which leaves the internal Screen Pixmap
> pointer pointing to the end of an allocation, where
> careless use can corrupt memory willy nilly.
>
> We didn't really have a compelling reason to do this;
> the qxl->fb memory can be dedicated to just screen use,
> and the host_image can manage it's own memory.
> ---
> src/qxl_driver.c | 2 +-
> src/qxl_surface.c | 8 +-------
> 2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> index dbb8ceb..dbb66b3 100644
> --- a/src/qxl_driver.c
> +++ b/src/qxl_driver.c
> @@ -1717,7 +1717,7 @@ qxl_fb_init (qxl_screen_t *qxl, ScreenPtr
> pScreen)
> ErrorF ("allocated %d x %d %p\n", pScrn->virtualX,
> pScrn->virtualY, qxl->fb);
> #endif
>
> - if (!fbScreenInit (pScreen, NULL,
> + if (!fbScreenInit (pScreen, qxl->fb,
> pScrn->virtualX, pScrn->virtualY,
> pScrn->xDpi, pScrn->yDpi,
> pScrn->displayWidth,
> pScrn->bitsPerPixel))
> diff --git a/src/qxl_surface.c b/src/qxl_surface.c
> index e88675f..76ec7d1 100644
> --- a/src/qxl_surface.c
> +++ b/src/qxl_surface.c
> @@ -404,15 +404,9 @@ qxl_surface_cache_create_primary
> (surface_cache_t *cache,
> dev_image = pixman_image_create_bits (format, mode->x_res,
> mode->y_res,
> (uint32_t *)dev_addr, -mode->stride);
>
> - if (qxl->fb != NULL)
> - free(qxl->fb);
> - qxl->fb = calloc (qxl->virtual_x * qxl->virtual_y, 4);
> - if (!qxl->fb)
> - return NULL;
> -
> host_image = pixman_image_create_bits (format,
> qxl->virtual_x, qxl->virtual_y,
> - qxl->fb, mode->stride);
> + NULL, mode->stride);
So we allocate another framebuffer here. Hmm, it is just another 1-16 MB of memory ;)
Do you have an example how to produce corruption?
Can you reuse the same buffer and still pass non NULL? I think it might not be trivial though, so in that case I'm ok with this since I don't see any other option.
> #if 0
> xf86DrvMsg(cache->qxl->pScrn->scrnIndex, X_ERROR,
> "testing dev_image memory (%d x %d)\n",
> --
> 1.7.10.4
>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel