libdrm: Fix some warnings reported by clang's scan-build tool

libdrm: Fix some warnings reported by clang's scan-build tool

[Johannes Obermayr]

Patches 1 to 4 were sent to mesa-dev.

[PATCH 1/5] libkms/intel.c: Fix a memory leak and a dead assignment as well as cleanup code a bit.

[Johannes Obermayr]

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

[PATCH 2/5] libkms/nouveau.c: Fix a memory leak and cleanup code a bit.

[Johannes Obermayr]

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

[PATCH 3/5] nouveau/nouveau.c: Fix two memory leaks.

[Johannes Obermayr]

---
 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;
 	}
 
-- 
1.7.7

[PATCH 4/5] xf86drm.c: Make more code UDEV unrelevant and fix a memory leak.

[Johannes Obermayr]

---
 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;
-- 
1.7.7

[PATCH 5/5] modetest.c: Add return 0 in bit_name_fn(res) macro.

[Johannes Obermayr]

---
 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[] = {
-- 
1.7.7

Re: libdrm: Fix some warnings reported by clang's scan-build tool

[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

libdrm: Fix some warnings reported by clang's scan-build tool [try 2]

[Johannes Obermayr]

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

[PATCH 1/6] libkms/intel.c: Fix a memory leak and a dead assignment as well as make some code easier to read.

[Johannes Obermayr]

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

[PATCH 2/6] libkms/nouveau.c: Fix a memory leak and make some code easier to read.

[Johannes Obermayr]

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

[PATCH 3/6] nouveau/nouveau.c: Fix two memory leaks.

[Johannes Obermayr]

---
 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;
 	}
 
-- 
1.7.7

[PATCH 4/6] xf86drm.c: Make more code UDEV unrelevant.

[Johannes Obermayr]

---
 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;
-- 
1.7.7

[PATCH 5/6] xf86drm.c: Fix two memory leaks.

[Johannes Obermayr]

---
 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;
-- 
1.7.7

[PATCH 6/6] modetest.c: Add return 0 in bit_name_fn(res) macro.

[Johannes Obermayr]

---
 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[] = {
-- 
1.7.7