Re: [PATCH v4 3/5] drm: Add dispatcher and driver identification for DRM

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2648 bytes --]

On Mon, Aug 24, 2015 at 02:42:48PM +0200, Patrik Jakobsson wrote:
> * Makefile.am: Add compilation of drm.c.
> * defs.h: Add extern declaration of drm_ioctl when drm headers are found.
> * drm.c: New file.
> * ioctl.c (ioctl_decode): Dispatch drm ioctls when drm headers are found.

* defs.h (drm_decode_number, drm_ioctl): New prototypes.
* drm.c: New file.
* Makefile.am (strace_SOURCES): Add it.
* ioctl.c (ioctl_decode_command_number, ioctl_decode)
[HAVE_DRM_H || HAVE_DRM_DRM_H]: Dispatch drm ioctls.

> --- a/defs.h
> +++ b/defs.h
> @@ -612,6 +612,9 @@ extern const char *sprint_open_modes(int);
>  extern void print_seccomp_filter(struct tcb *tcp, unsigned long);
>  
>  extern int block_ioctl(struct tcb *, const unsigned int, long);
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)
> +extern int drm_ioctl(struct tcb *, const unsigned int, long);
> +#endif

I think function these prototypes could be added unconditionally.
Note that v3 version of this patch also declared drm_decode_number().

> --- /dev/null
> +++ b/drm.c
[...]
> +#include "defs.h"
> +
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)
> +
> +#ifdef HAVE_DRM_H
> +#include <drm.h>
> +#else
> +#include <drm/drm.h>
> +#endif
> +
> +#include <sys/param.h>

Let's include <sys/param.h> before drm stuff.

> +#define DRM_MAX_NAME_LEN 128
> +
> +inline int drm_is_priv(const unsigned int num)
> +{
> +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> +		_IOC_NR(num) < DRM_COMMAND_END);
> +}

This function has to be static, and like other static functions,
it has to be added along with its first user, otherwise the project
won't build with --enable-gcc-Werror.

> +static char *drm_get_driver_name(struct tcb *tcp)
> +{
> +	char path[PATH_MAX];
> +	char link[PATH_MAX];
> +	int ret;
> +
> +	if (getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1) < 0)
> +		return NULL;
> +
> +	snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver",
> +		 basename(path));

if (snprintf(link, sizeof(link), ...) >= sizeof(link))
	return NULL;

> +static void drm_free_priv(void *data)
> +{
> +	free(data);
> +}

Do we really need a wrapper for free(3)?

> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -248,6 +248,10 @@ ioctl_decode(struct tcb *tcp)
>  	case 0x22:
>  		return scsi_ioctl(tcp, code, arg);
>  #endif
> +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H)
> +	case 'd':
> +		return drm_ioctl(tcp, code, arg);
> +#endif
>  	case 'L':
>  		return loop_ioctl(tcp, code, arg);
>  	case 'M':

v3 version also patched ioctl_decode_command_number()
to call drm_decode_number().


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx