--- libkms/intel.c | 25 ++++++++++++++----------- 1 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/libkms/intel.c b/libkms/intel.c index 8b8249b..b8ac343 100644 --- a/libkms/intel.c +++ b/libkms/intel.c @@ -93,14 +93,18 @@ intel_bo_create(struct kms_driver *kms, if (!bo) return -ENOMEM;
- if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) { + switch (type) { + case KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8: pitch = 64 * 4; size = 64 * 64 * 4; - } else if (type == KMS_BO_TYPE_SCANOUT_X8R8G8B8) { + break; + case KMS_BO_TYPE_SCANOUT_X8R8G8B8: pitch = width * 4; pitch = (pitch + 512 - 1) & ~(512 - 1); size = pitch * ((height + 4 - 1) & ~(4 - 1)); - } else { + break; + default: + free(bo); return -EINVAL; }
@@ -108,8 +112,10 @@ intel_bo_create(struct kms_driver *kms, arg.size = size;
ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_CREATE, &arg, sizeof(arg)); - if (ret) - goto err_free; + if (ret) { + free(bo); + return ret; + }
bo->base.kms = kms; bo->base.handle = arg.handle; @@ -124,21 +130,18 @@ intel_bo_create(struct kms_driver *kms, tile.handle = bo->base.handle; tile.tiling_mode = I915_TILING_X; tile.stride = bo->base.pitch; - - ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); #if 0 + ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); if (ret) { kms_bo_destroy(out); return ret; } +#else + drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); #endif }
return 0; - -err_free: - free(bo); - return ret; }
static int
--- libkms/nouveau.c | 20 +++++++++++--------- 1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/libkms/nouveau.c b/libkms/nouveau.c index 0e24a15..4cbca96 100644 --- a/libkms/nouveau.c +++ b/libkms/nouveau.c @@ -94,14 +94,18 @@ nouveau_bo_create(struct kms_driver *kms, if (!bo) return -ENOMEM;
- if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) { + switch (type) { + case KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8: pitch = 64 * 4; size = 64 * 64 * 4; - } else if (type == KMS_BO_TYPE_SCANOUT_X8R8G8B8) { + break; + case KMS_BO_TYPE_SCANOUT_X8R8G8B8: pitch = width * 4; pitch = (pitch + 512 - 1) & ~(512 - 1); size = pitch * height; - } else { + break; + default: + free(bo); return -EINVAL; }
@@ -114,8 +118,10 @@ nouveau_bo_create(struct kms_driver *kms, arg.channel_hint = 0;
ret = drmCommandWriteRead(kms->fd, DRM_NOUVEAU_GEM_NEW, &arg, sizeof(arg)); - if (ret) - goto err_free; + if (ret) { + free(bo); + return ret; + }
bo->base.kms = kms; bo->base.handle = arg.info.handle; @@ -126,10 +132,6 @@ nouveau_bo_create(struct kms_driver *kms, *out = &bo->base;
return 0; - -err_free: - free(bo); - return ret; }
static int
--- nouveau/nouveau.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c index 5aa4107..e91287f 100644 --- a/nouveau/nouveau.c +++ b/nouveau/nouveau.c @@ -95,6 +95,7 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) (dev->drm_version < 0x01000000 || dev->drm_version >= 0x02000000)) { nouveau_device_del(&dev); + free(nvdev); return -EINVAL; }
@@ -105,6 +106,7 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_AGP_SIZE, &gart); if (ret) { nouveau_device_del(&dev); + free(nvdev); return ret; }
--- xf86drm.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c index 6ea068f..e3789c8 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -255,6 +255,7 @@ static int drmMatchBusID(const char *id1, const char *id2, int pci_domain_ok) return 0; }
+#if !defined(UDEV) /** * Handles error checking for chown call. * @@ -284,6 +285,7 @@ static int chown_check_return(const char *path, uid_t owner, gid_t group) path, errno, strerror(errno)); return -1; } +#endif
/** * Open the DRM device, creating it if necessary. @@ -303,14 +305,17 @@ static int drmOpenDevice(long dev, int minor, int type) stat_t st; char buf[64]; int fd; +#if !defined(UDEV) mode_t devmode = DRM_DEV_MODE, serv_mode; int isroot = !geteuid(); uid_t user = DRM_DEV_UID; gid_t group = DRM_DEV_GID, serv_group; - +#endif + sprintf(buf, type ? DRM_DEV_NAME : DRM_CONTROL_DEV_NAME, DRM_DIR_NAME, minor); drmMsg("drmOpenDevice: node name is %s\n", buf);
+#if !defined(UDEV) if (drm_server_info) { drm_server_info->get_perms(&serv_group, &serv_mode); devmode = serv_mode ? serv_mode : DRM_DEV_MODE; @@ -318,7 +323,6 @@ static int drmOpenDevice(long dev, int minor, int type) group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; }
-#if !defined(UDEV) if (stat(DRM_DIR_NAME, &st)) { if (!isroot) return DRM_ERR_NOT_ROOT; @@ -1395,8 +1399,10 @@ drm_context_t *drmGetReservedContextList(int fd, int *count) }
res.contexts = list; - if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res)) + if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res)) { + drmFree(retval); return NULL; + }
for (i = 0; i < res.count; i++) retval[i] = list[i].handle;
--- tests/modetest/modetest.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index ec3121e..00129fa 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -128,6 +128,7 @@ char * res##_str(int type) { \ sep = ", "; \ } \ } \ + return 0; \ }
static const char *mode_type_names[] = {
Am Freitag, 13. Juli 2012, 18:47:50 schrieb Marcin Slusarz:
On Fri, Jul 13, 2012 at 05:49:12PM +0200, Johannes Obermayr wrote:
Patches 1 to 4 were sent to mesa-dev.
And you chose to ignore most of my comments. Fine. Don't expect further reviews from me.
Marcin
Patch 1 and 2: - Adapted - I want to keep proposed easier to read "switch" case
Patch 3: - Resend - Waiting on your response: http://lists.freedesktop.org/archives/mesa-dev/2012-June/023456.html
Patch 4 and 5: - Splitted - http://llvm.org/bugs/show_bug.cgi?id=13358 (forgot to split and to add 'drmFree(list);') - The 'more if's case' seems better to me
Patch 6: - Resend
Marcin, not that I ignore comments. But sometimes I want to hear also opinions from (some more) other people. I hope I can calm the waves ...
Johannes
--- libkms/intel.c | 32 +++++++++++++++++--------------- 1 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/libkms/intel.c b/libkms/intel.c index 8b8249b..12175b0 100644 --- a/libkms/intel.c +++ b/libkms/intel.c @@ -89,27 +89,32 @@ intel_bo_create(struct kms_driver *kms, } }
- bo = calloc(1, sizeof(*bo)); - if (!bo) - return -ENOMEM; - - if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) { + switch (type) { + case KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8: pitch = 64 * 4; size = 64 * 64 * 4; - } else if (type == KMS_BO_TYPE_SCANOUT_X8R8G8B8) { + break; + case KMS_BO_TYPE_SCANOUT_X8R8G8B8: pitch = width * 4; pitch = (pitch + 512 - 1) & ~(512 - 1); size = pitch * ((height + 4 - 1) & ~(4 - 1)); - } else { + break; + default: return -EINVAL; }
+ bo = calloc(1, sizeof(*bo)); + if (!bo) + return -ENOMEM; + memset(&arg, 0, sizeof(arg)); arg.size = size;
ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_CREATE, &arg, sizeof(arg)); - if (ret) - goto err_free; + if (ret) { + free(bo); + return ret; + }
bo->base.kms = kms; bo->base.handle = arg.handle; @@ -124,21 +129,18 @@ intel_bo_create(struct kms_driver *kms, tile.handle = bo->base.handle; tile.tiling_mode = I915_TILING_X; tile.stride = bo->base.pitch; - - ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); #if 0 + ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); if (ret) { kms_bo_destroy(out); return ret; } +#else + drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile)); #endif }
return 0; - -err_free: - free(bo); - return ret; }
static int
--- libkms/nouveau.c | 27 ++++++++++++++------------- 1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/libkms/nouveau.c b/libkms/nouveau.c index 0e24a15..fbca6fe 100644 --- a/libkms/nouveau.c +++ b/libkms/nouveau.c @@ -90,21 +90,24 @@ nouveau_bo_create(struct kms_driver *kms, } }
- bo = calloc(1, sizeof(*bo)); - if (!bo) - return -ENOMEM; - - if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) { + switch (type) { + case KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8: pitch = 64 * 4; size = 64 * 64 * 4; - } else if (type == KMS_BO_TYPE_SCANOUT_X8R8G8B8) { + break; + case KMS_BO_TYPE_SCANOUT_X8R8G8B8: pitch = width * 4; pitch = (pitch + 512 - 1) & ~(512 - 1); size = pitch * height; - } else { + break; + default: return -EINVAL; }
+ bo = calloc(1, sizeof(*bo)); + if (!bo) + return -ENOMEM; + memset(&arg, 0, sizeof(arg)); arg.info.size = size; arg.info.domain = NOUVEAU_GEM_DOMAIN_MAPPABLE | NOUVEAU_GEM_DOMAIN_VRAM; @@ -114,8 +117,10 @@ nouveau_bo_create(struct kms_driver *kms, arg.channel_hint = 0;
ret = drmCommandWriteRead(kms->fd, DRM_NOUVEAU_GEM_NEW, &arg, sizeof(arg)); - if (ret) - goto err_free; + if (ret) { + free(bo); + return ret; + }
bo->base.kms = kms; bo->base.handle = arg.info.handle; @@ -126,10 +131,6 @@ nouveau_bo_create(struct kms_driver *kms, *out = &bo->base;
return 0; - -err_free: - free(bo); - return ret; }
static int
--- nouveau/nouveau.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c index 5aa4107..e91287f 100644 --- a/nouveau/nouveau.c +++ b/nouveau/nouveau.c @@ -95,6 +95,7 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) (dev->drm_version < 0x01000000 || dev->drm_version >= 0x02000000)) { nouveau_device_del(&dev); + free(nvdev); return -EINVAL; }
@@ -105,6 +106,7 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev) ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_AGP_SIZE, &gart); if (ret) { nouveau_device_del(&dev); + free(nvdev); return ret; }
--- xf86drm.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c index 6ea068f..e652731 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -255,6 +255,7 @@ static int drmMatchBusID(const char *id1, const char *id2, int pci_domain_ok) return 0; }
+#if !defined(UDEV) /** * Handles error checking for chown call. * @@ -284,6 +285,7 @@ static int chown_check_return(const char *path, uid_t owner, gid_t group) path, errno, strerror(errno)); return -1; } +#endif
/** * Open the DRM device, creating it if necessary. @@ -303,14 +305,17 @@ static int drmOpenDevice(long dev, int minor, int type) stat_t st; char buf[64]; int fd; +#if !defined(UDEV) mode_t devmode = DRM_DEV_MODE, serv_mode; int isroot = !geteuid(); uid_t user = DRM_DEV_UID; gid_t group = DRM_DEV_GID, serv_group; - +#endif + sprintf(buf, type ? DRM_DEV_NAME : DRM_CONTROL_DEV_NAME, DRM_DIR_NAME, minor); drmMsg("drmOpenDevice: node name is %s\n", buf);
+#if !defined(UDEV) if (drm_server_info) { drm_server_info->get_perms(&serv_group, &serv_mode); devmode = serv_mode ? serv_mode : DRM_DEV_MODE; @@ -318,7 +323,6 @@ static int drmOpenDevice(long dev, int minor, int type) group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; }
-#if !defined(UDEV) if (stat(DRM_DIR_NAME, &st)) { if (!isroot) return DRM_ERR_NOT_ROOT;
--- xf86drm.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c index e652731..c1cc170 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -1399,8 +1399,11 @@ drm_context_t *drmGetReservedContextList(int fd, int *count) }
res.contexts = list; - if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res)) + if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res)) { + drmFree(list); + drmFree(retval); return NULL; + }
for (i = 0; i < res.count; i++) retval[i] = list[i].handle;
--- tests/modetest/modetest.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index ec3121e..00129fa 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -128,6 +128,7 @@ char * res##_str(int type) { \ sep = ", "; \ } \ } \ + return 0; \ }
static const char *mode_type_names[] = {
dri-devel@lists.freedesktop.org