On Thu, Jun 28, 2012 at 09:51:58PM +0200, Johannes Obermayr wrote:

These patches should be sent to dri-devel, not mesa-dev.

---

xf86drm.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c index 6ea068f..798f1fd 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,13 +305,15 @@ static int drmOpenDevice(long dev, int minor, int type) stat_t st; char buf[64]; int fd;

• 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) 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;

• sprintf(buf, type ? DRM_DEV_NAME : DRM_CONTROL_DEV_NAME, DRM_DIR_NAME, minor);

• drmMsg("drmOpenDevice: node name is %s\n", buf);

  if (drm_server_info) { drm_server_info->get_perms(&serv_group, &serv_mode);

@@ -318,7 +322,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;

You should not mix code with declarations. However, UDEV and non-UDEV codepaths share very little code. I'm wondering whether it would be better to organize it like:

static int drmOpenDevice(long dev, int minor, int type) { #if defined(UDEV) ... #else ... #endif }

@@ -1395,8 +1398,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;

--

This is not enough. list will leak too.

Make it a separate patch please.

Marcin