Re: [bug report] drm/prime: replace NULL with error value in drm_prime_pages_to_sg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/15/2018 06:22 AM, YoungJun Cho wrote:
Dear Dan,

Your mail flashes back to my memory 5 years ago.
Back then, I had cleaned up the exynos driver.

And the replacement IS_ERR was one of items.

IMHO it is still better to modify those two functions, drm_gem_cma_prime_get_sg_table and xen_drm_front_gem_get_sg_table.

Thank you.
Best regards YJ


On Thu, 14 Jun 2018, 23:42 Dan Carpenter, <dan.carpenter@xxxxxxxxxx <mailto:dan.carpenter@xxxxxxxxxx>> wrote:

    Hello YoungJun Cho,

    The patch 7e3d88f9cce3: "drm/prime: replace NULL with error value in
    drm_prime_pages_to_sg" from Jun 24, 2013, leads to the following
    static checker warning:

            drivers/gpu/drm/drm_prime.c:317 drm_gem_map_dma_buf()
            warn: 'sgt' can also be NULL

    drivers/gpu/drm/drm_prime.c
       307          /*
       308           * two mappings with different directions for the
    same attachment are
       309           * not allowed
       310           */
       311          if (WARN_ON(prime_attach->dir != DMA_NONE))
       312                  return ERR_PTR(-EBUSY);
       313
       314          sgt = obj->dev->driver->gem_prime_get_sg_table(obj);

    The problematic functions here are
    drm_gem_cma_prime_get_sg_table() and
    xen_drm_front_gem_get_sg_table().  My preference would be to update
    those functions to return error pointers, but I'm not familiar
    with the
    code or able to test it.

I believe it is safe to change the return value to error pointer:
the code below (which is the only caller of .gem_prime_get_sg_table
callback) does not check for NULL anyways

Dan, do you want me to send the patch for Xen or you prefer to post both
Xen and CMA helpers fix yourself?

    But this static checker test seems pretty good so I'm likely to
    publish
    it soon and then this sort of bug will normally be caught quickly.

       315
       316          if (!IS_ERR(sgt)) {
       317                  if (!dma_map_sg_attrs(attach->dev,
    sgt->sgl, sgt->nents, dir,
       318 DMA_ATTR_SKIP_CPU_SYNC)) {
       319                          sg_free_table(sgt);
       320                          kfree(sgt);
       321                          sgt = ERR_PTR(-ENOMEM);
       322                  } else {
       323                          prime_attach->sgt = sgt;
       324                          prime_attach->dir = dir;
       325                  }
       326          }
       327
       328          return sgt;

    regards,
    dan carpenter
    _______________________________________________
    dri-devel mailing list
    dri-devel@xxxxxxxxxxxxxxxxxxxxx
    <mailto:dri-devel@xxxxxxxxxxxxxxxxxxxxx>
    https://lists.freedesktop.org/mailman/listinfo/dri-devel



_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux