On Tue, Sep 08, 2015 at 03:36:25AM +0300, Dmitry V. Levin wrote:
> 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.
Ugh, thought I had this correct already. Will fix.
> > --- 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().
Ok. Will move the number decoding pieces back into this patch. Not sure why I
did this in the first place.
> > --- /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.
Ok
> > +#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.
Will move it to the correct patch and make it static.
> > +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;
>
According to manpage snprintf never returns more than the specified size - 1.
So the only error we can check for is:
if (snprintf(link, sizeof(link), ...) <
sizeof("/sys/class/drm/%s/device/driver"))
> > +static void drm_free_priv(void *data)
> > +{
> > + free(data);
> > +}
>
> Do we really need a wrapper for free(3)?
No. The only reason I see is for clarity on how to use the tcb priv interface.
But that is ofc debatable.
> > --- 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().
Will move it back into this patch.
>
>
> --
> ldv
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
