From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Vesely Subject: Re: [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs Date: Wed, 11 Feb 2015 10:57:08 -0500 Message-ID: <1423670228.3926.52.camel@rutgers.edu> References: <1423654968-10553-1-git-send-email-daniel.vetter@ffwll.ch> <1423654968-10553-4-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0446397706==" Return-path: Received: from mail-qg0-f48.google.com (mail-qg0-f48.google.com [209.85.192.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 077216E29F for ; Wed, 11 Feb 2015 07:57:11 -0800 (PST) Received: by mail-qg0-f48.google.com with SMTP id a108so3289436qge.7 for ; Wed, 11 Feb 2015 07:57:11 -0800 (PST) In-Reply-To: <1423654968-10553-4-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , DRI Development List-Id: dri-devel@lists.freedesktop.org --===============0446397706== Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-+gJ+VlGcbDOhan6s7ILw" --=-+gJ+VlGcbDOhan6s7ILw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2015-02-11 at 12:42 +0100, Daniel Vetter wrote: > We really have to do this to avoid surprises when extending the ABI > later on. Especially when growing the structures. >=20 > A bit overkill to update all the old legacy ioctl wrappers, but can't > hurt really either. >=20 > Signed-off-by: Daniel Vetter > --- > xf86drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++--------------= ------ > 1 file changed, 77 insertions(+), 35 deletions(-) >=20 > diff --git a/xf86drm.c b/xf86drm.c > index 263d6835c29a..a2e24eb5f76c 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -89,6 +89,8 @@ > #define DRM_NODE_PRIMARY 1 > #define DRM_NODE_RENDER 2 > =20 > +#define memclear(s) memset(&s, 0, sizeof(s)) > + > static drmServerInfoPtr drm_server_info; > =20 > void drmSetServerInfo(drmServerInfoPtr info) > @@ -766,12 +768,7 @@ drmVersionPtr drmGetVersion(int fd) > drmVersionPtr retval; > drm_version_t *version =3D drmMalloc(sizeof(*version)); > =20 > - version->name_len =3D 0; > - version->name =3D NULL; > - version->date_len =3D 0; > - version->date =3D NULL; > - version->desc_len =3D 0; > - version->desc =3D NULL; > + memclear(version); I think this should be memclear(*version). Otherwise it clears the pointer not the structure. > =20 > if (drmIoctl(fd, DRM_IOCTL_VERSION, version)) { > drmFreeKernelVersion(version); > @@ -839,9 +836,12 @@ drmVersionPtr drmGetLibVersion(int fd) > =20 > int drmGetCap(int fd, uint64_t capability, uint64_t *value) > { > - struct drm_get_cap cap =3D { capability, 0 }; > + struct drm_get_cap cap; > int ret; > =20 > + memclear(cap); > + cap.capability =3D capability; > + > ret =3D drmIoctl(fd, DRM_IOCTL_GET_CAP, &cap); > if (ret) > return ret; > @@ -852,7 +852,11 @@ int drmGetCap(int fd, uint64_t capability, uint64_t = *value) > =20 > int drmSetClientCap(int fd, uint64_t capability, uint64_t value) > { > - struct drm_set_client_cap cap =3D { capability, value }; > + struct drm_set_client_cap cap; > + > + memclear(cap); > + cap.capability =3D capability; > + cap.value =3D value; > =20 > return drmIoctl(fd, DRM_IOCTL_SET_CLIENT_CAP, &cap); > } > @@ -887,8 +891,7 @@ char *drmGetBusid(int fd) > { > drm_unique_t u; > =20 > - u.unique_len =3D 0; > - u.unique =3D NULL; > + memclear(u); > =20 > if (drmIoctl(fd, DRM_IOCTL_GET_UNIQUE, &u)) > return NULL; > @@ -917,6 +920,7 @@ int drmSetBusid(int fd, const char *busid) > { > drm_unique_t u; > =20 > + memclear(u); > u.unique =3D (char *)busid; > u.unique_len =3D strlen(busid); > =20 > @@ -930,6 +934,8 @@ int drmGetMagic(int fd, drm_magic_t * magic) > { > drm_auth_t auth; > =20 > + memclear(auth); > + > *magic =3D 0; > if (drmIoctl(fd, DRM_IOCTL_GET_MAGIC, &auth)) > return -errno; > @@ -941,6 +947,7 @@ int drmAuthMagic(int fd, drm_magic_t magic) > { > drm_auth_t auth; > =20 > + memclear(auth); > auth.magic =3D magic; > if (drmIoctl(fd, DRM_IOCTL_AUTH_MAGIC, &auth)) > return -errno; > @@ -1002,9 +1009,9 @@ int drmAddMap(int fd, drm_handle_t offset, drmSize = size, drmMapType type, > { > drm_map_t map; > =20 > + memclear(map); > map.offset =3D offset; > map.size =3D size; > - map.handle =3D 0; > map.type =3D type; > map.flags =3D flags; > if (drmIoctl(fd, DRM_IOCTL_ADD_MAP, &map)) > @@ -1018,6 +1025,7 @@ int drmRmMap(int fd, drm_handle_t handle) > { > drm_map_t map; > =20 > + memclear(map); > map.handle =3D (void *)(uintptr_t)handle; > =20 > if(drmIoctl(fd, DRM_IOCTL_RM_MAP, &map)) > @@ -1046,10 +1054,9 @@ int drmAddBufs(int fd, int count, int size, drmBuf= DescFlags flags, > { > drm_buf_desc_t request; > =20 > + memclear(request); > request.count =3D count; > request.size =3D size; > - request.low_mark =3D 0; > - request.high_mark =3D 0; > request.flags =3D flags; > request.agp_start =3D agp_offset; > =20 > @@ -1063,8 +1070,7 @@ int drmMarkBufs(int fd, double low, double high) > drm_buf_info_t info; > int i; > =20 > - info.count =3D 0; > - info.list =3D NULL; > + memclear(info); > =20 > if (drmIoctl(fd, DRM_IOCTL_INFO_BUFS, &info)) > return -EINVAL; > @@ -1114,6 +1120,7 @@ int drmFreeBufs(int fd, int count, int *list) > { > drm_buf_free_t request; > =20 > + memclear(request); > request.count =3D count; > request.list =3D list; > if (drmIoctl(fd, DRM_IOCTL_FREE_BUFS, &request)) > @@ -1202,8 +1209,7 @@ drmBufInfoPtr drmGetBufInfo(int fd) > drmBufInfoPtr retval; > int i; > =20 > - info.count =3D 0; > - info.list =3D NULL; > + memclear(info); > =20 > if (drmIoctl(fd, DRM_IOCTL_INFO_BUFS, &info)) > return NULL; > @@ -1253,9 +1259,7 @@ drmBufMapPtr drmMapBufs(int fd) > drmBufMapPtr retval; > int i; > =20 > - bufs.count =3D 0; > - bufs.list =3D NULL; > - bufs.virtual =3D NULL; > + memclear(bufs); > if (drmIoctl(fd, DRM_IOCTL_MAP_BUFS, &bufs)) > return NULL; > =20 > @@ -1371,6 +1375,7 @@ int drmGetLock(int fd, drm_context_t context, drmLo= ckFlags flags) > { > drm_lock_t lock; > =20 > + memclear(lock); > lock.context =3D context; > lock.flags =3D 0; > if (flags & DRM_LOCK_READY) lock.flags |=3D _DRM_LOCK_READY; > @@ -1401,8 +1406,8 @@ int drmUnlock(int fd, drm_context_t context) > { > drm_lock_t lock; > =20 > + memclear(lock); > lock.context =3D context; > - lock.flags =3D 0; > return drmIoctl(fd, DRM_IOCTL_UNLOCK, &lock); > } > =20 > @@ -1413,8 +1418,7 @@ drm_context_t *drmGetReservedContextList(int fd, in= t *count) > drm_context_t * retval; > int i; > =20 > - res.count =3D 0; > - res.contexts =3D NULL; > + memclear(res); > if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res)) > return NULL; > =20 > @@ -1467,7 +1471,7 @@ int drmCreateContext(int fd, drm_context_t *handle) > { > drm_ctx_t ctx; > =20 > - ctx.flags =3D 0; /* Modified with functions below */ > + memclear(ctx); > if (drmIoctl(fd, DRM_IOCTL_ADD_CTX, &ctx)) > return -errno; > *handle =3D ctx.handle; > @@ -1478,6 +1482,7 @@ int drmSwitchToContext(int fd, drm_context_t contex= t) > { > drm_ctx_t ctx; > =20 > + memclear(ctx); > ctx.handle =3D context; > if (drmIoctl(fd, DRM_IOCTL_SWITCH_CTX, &ctx)) > return -errno; > @@ -1494,8 +1499,8 @@ int drmSetContextFlags(int fd, drm_context_t contex= t, drm_context_tFlags flags) > * X server (which promises to maintain hardware context), or in the > * client-side library when buffers are swapped on behalf of two thr= eads. > */ > + memclear(ctx); > ctx.handle =3D context; > - ctx.flags =3D 0; > if (flags & DRM_CONTEXT_PRESERVED) > ctx.flags |=3D _DRM_CONTEXT_PRESERVED; > if (flags & DRM_CONTEXT_2DONLY) > @@ -1510,6 +1515,7 @@ int drmGetContextFlags(int fd, drm_context_t contex= t, > { > drm_ctx_t ctx; > =20 > + memclear(ctx); > ctx.handle =3D context; > if (drmIoctl(fd, DRM_IOCTL_GET_CTX, &ctx)) > return -errno; > @@ -1541,6 +1547,8 @@ int drmGetContextFlags(int fd, drm_context_t contex= t, > int drmDestroyContext(int fd, drm_context_t handle) > { > drm_ctx_t ctx; > + > + memclear(ctx); > ctx.handle =3D handle; > if (drmIoctl(fd, DRM_IOCTL_RM_CTX, &ctx)) > return -errno; > @@ -1550,6 +1558,8 @@ int drmDestroyContext(int fd, drm_context_t handle) > int drmCreateDrawable(int fd, drm_drawable_t *handle) > { > drm_draw_t draw; > + > + memclear(draw); > if (drmIoctl(fd, DRM_IOCTL_ADD_DRAW, &draw)) > return -errno; > *handle =3D draw.handle; > @@ -1559,6 +1569,8 @@ int drmCreateDrawable(int fd, drm_drawable_t *handl= e) > int drmDestroyDrawable(int fd, drm_drawable_t handle) > { > drm_draw_t draw; > + > + memclear(draw); > draw.handle =3D handle; > if (drmIoctl(fd, DRM_IOCTL_RM_DRAW, &draw)) > return -errno; > @@ -1571,6 +1583,7 @@ int drmUpdateDrawableInfo(int fd, drm_drawable_t ha= ndle, > { > drm_update_draw_t update; > =20 > + memclear(update); > update.handle =3D handle; > update.type =3D type; > update.num =3D num; > @@ -1636,6 +1649,7 @@ int drmAgpEnable(int fd, unsigned long mode) > { > drm_agp_mode_t m; > =20 > + memclear(mode); > m.mode =3D mode; > if (drmIoctl(fd, DRM_IOCTL_AGP_ENABLE, &m)) > return -errno; > @@ -1664,9 +1678,9 @@ int drmAgpAlloc(int fd, unsigned long size, unsigne= d long type, > { > drm_agp_buffer_t b; > =20 > + memclear(b); > *handle =3D DRM_AGP_NO_HANDLE; > b.size =3D size; > - b.handle =3D 0; > b.type =3D type; > if (drmIoctl(fd, DRM_IOCTL_AGP_ALLOC, &b)) > return -errno; > @@ -1693,7 +1707,7 @@ int drmAgpFree(int fd, drm_handle_t handle) > { > drm_agp_buffer_t b; > =20 > - b.size =3D 0; > + memclear(b); > b.handle =3D handle; > if (drmIoctl(fd, DRM_IOCTL_AGP_FREE, &b)) > return -errno; > @@ -1718,6 +1732,7 @@ int drmAgpBind(int fd, drm_handle_t handle, unsigne= d long offset) > { > drm_agp_binding_t b; > =20 > + memclear(b); > b.handle =3D handle; > b.offset =3D offset; > if (drmIoctl(fd, DRM_IOCTL_AGP_BIND, &b)) > @@ -1742,8 +1757,8 @@ int drmAgpUnbind(int fd, drm_handle_t handle) > { > drm_agp_binding_t b; > =20 > + memclear(b); > b.handle =3D handle; > - b.offset =3D 0; > if (drmIoctl(fd, DRM_IOCTL_AGP_UNBIND, &b)) > return -errno; > return 0; > @@ -1765,6 +1780,8 @@ int drmAgpVersionMajor(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return -errno; > return i.agp_version_major; > @@ -1786,6 +1803,8 @@ int drmAgpVersionMinor(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return -errno; > return i.agp_version_minor; > @@ -1807,6 +1826,8 @@ unsigned long drmAgpGetMode(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.mode; > @@ -1828,6 +1849,8 @@ unsigned long drmAgpBase(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.aperture_base; > @@ -1849,6 +1872,8 @@ unsigned long drmAgpSize(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.aperture_size; > @@ -1870,6 +1895,8 @@ unsigned long drmAgpMemoryUsed(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.memory_used; > @@ -1891,6 +1918,8 @@ unsigned long drmAgpMemoryAvail(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.memory_allowed; > @@ -1912,6 +1941,8 @@ unsigned int drmAgpVendorId(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.id_vendor; > @@ -1933,6 +1964,8 @@ unsigned int drmAgpDeviceId(int fd) > { > drm_agp_info_t i; > =20 > + memclear(i); > + > if (drmIoctl(fd, DRM_IOCTL_AGP_INFO, &i)) > return 0; > return i.id_device; > @@ -1942,9 +1975,10 @@ int drmScatterGatherAlloc(int fd, unsigned long si= ze, drm_handle_t *handle) > { > drm_scatter_gather_t sg; > =20 > + memclear(sg); > + > *handle =3D 0; > sg.size =3D size; > - sg.handle =3D 0; > if (drmIoctl(fd, DRM_IOCTL_SG_ALLOC, &sg)) > return -errno; > *handle =3D sg.handle; > @@ -1955,7 +1989,7 @@ int drmScatterGatherFree(int fd, drm_handle_t handl= e) > { > drm_scatter_gather_t sg; > =20 > - sg.size =3D 0; > + memclear(sg); > sg.handle =3D handle; > if (drmIoctl(fd, DRM_IOCTL_SG_FREE, &sg)) > return -errno; > @@ -2046,6 +2080,7 @@ int drmCtlInstHandler(int fd, int irq) > { > drm_control_t ctl; > =20 > + memclear(ctl); > ctl.func =3D DRM_INST_HANDLER; > ctl.irq =3D irq; > if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl)) > @@ -2069,6 +2104,7 @@ int drmCtlUninstHandler(int fd) > { > drm_control_t ctl; > =20 > + memclear(ctl); > ctl.func =3D DRM_UNINST_HANDLER; > ctl.irq =3D 0; > if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl)) > @@ -2080,8 +2116,8 @@ int drmFinish(int fd, int context, drmLockFlags fla= gs) > { > drm_lock_t lock; > =20 > + memclear(lock); > lock.context =3D context; > - lock.flags =3D 0; > if (flags & DRM_LOCK_READY) lock.flags |=3D _DRM_LOCK_READY; > if (flags & DRM_LOCK_QUIESCENT) lock.flags |=3D _DRM_LOCK_QUIESCENT= ; > if (flags & DRM_LOCK_FLUSH) lock.flags |=3D _DRM_LOCK_FLUSH; > @@ -2111,6 +2147,7 @@ int drmGetInterruptFromBusID(int fd, int busnum, in= t devnum, int funcnum) > { > drm_irq_busid_t p; > =20 > + memclear(p); > p.busnum =3D busnum; > p.devnum =3D devnum; > p.funcnum =3D funcnum; > @@ -2153,6 +2190,7 @@ int drmAddContextPrivateMapping(int fd, drm_context= _t ctx_id, > { > drm_ctx_priv_map_t map; > =20 > + memclear(map); > map.ctx_id =3D ctx_id; > map.handle =3D (void *)(uintptr_t)handle; > =20 > @@ -2166,6 +2204,7 @@ int drmGetContextPrivateMapping(int fd, drm_context= _t ctx_id, > { > drm_ctx_priv_map_t map; > =20 > + memclear(map); > map.ctx_id =3D ctx_id; > =20 > if (drmIoctl(fd, DRM_IOCTL_GET_SAREA_CTX, &map)) > @@ -2182,6 +2221,7 @@ int drmGetMap(int fd, int idx, drm_handle_t *offset= , drmSize *size, > { > drm_map_t map; > =20 > + memclear(map); > map.offset =3D idx; > if (drmIoctl(fd, DRM_IOCTL_GET_MAP, &map)) > return -errno; > @@ -2199,6 +2239,7 @@ int drmGetClient(int fd, int idx, int *auth, int *p= id, int *uid, > { > drm_client_t client; > =20 > + memclear(client); > client.idx =3D idx; > if (drmIoctl(fd, DRM_IOCTL_GET_CLIENT, &client)) > return -errno; > @@ -2215,6 +2256,7 @@ int drmGetStats(int fd, drmStatsT *stats) > drm_stats_t s; > unsigned i; > =20 > + memclear(s); > if (drmIoctl(fd, DRM_IOCTL_GET_STATS, &s)) > return -errno; > =20 > @@ -2352,6 +2394,7 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *v= ersion) > int retcode =3D 0; > drm_set_version_t sv; > =20 > + memclear(sv); > sv.drm_di_major =3D version->drm_di_major; > sv.drm_di_minor =3D version->drm_di_minor; > sv.drm_dd_major =3D version->drm_dd_major; > @@ -2383,12 +2426,11 @@ int drmSetInterfaceVersion(int fd, drmSetVersion = *version) > */ > int drmCommandNone(int fd, unsigned long drmCommandIndex) > { > - void *data =3D NULL; /* dummy */ > unsigned long request; > =20 > request =3D DRM_IO( DRM_COMMAND_BASE + drmCommandIndex); > =20 > - if (drmIoctl(fd, request, data)) { > + if (drmIoctl(fd, request, NULL)) { > return -errno; > } > return 0; > @@ -2543,12 +2585,12 @@ void drmCloseOnce(int fd) > =20 > int drmSetMaster(int fd) > { > - return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0); > + return drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL); > } > =20 > int drmDropMaster(int fd) > { > - return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0); > + return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL); > } > =20 > char *drmGetDeviceNameFromFd(int fd) --=20 Jan Vesely --=-+gJ+VlGcbDOhan6s7ILw Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCgAGBQJU23vVAAoJEDhUsXb6ZiH/W3wP/3fAuuNZ1k33etrfB9yqwDOu HmyYjlQQo+HoRyclTfOiYq+uadramD+elppif8Tbt8748cTWfbfVmSZTE23n+47z yj3YqREHNBleHMTBbqHRx+JHmBbeQd0yYyf3J7p7hu3UtO+0NOBBkPIBWfWF0skg +LUSSNcYBxvm10ibmLhjqNGFvShZUkIZUw6fuDI7tM61zkYRoefs4nU5v5jlNU/4 1AO48i5xeGQFUG8mXDnIs+17o+6IOnIdoPPrLkFtjyGP6nYnFs4Q2soc1gCWmiY1 BlF1JaaU7cK3I60t/ugpxQ5vJUOTgUtTqbEfK2EssJesi+KRAwcA2tzKW2K0hQjP 0Au2V8mVGI8U4ftFZ/Xw0FXXi0wJ2S0u+hZjRDj2DBJInCAlsr6jep6fv4id1hy7 wvUR0z3iXqrDK/AnV8+ioEbV3SV7FO5m6RI6ktO9833xCLdX77CiY6IRi2yn3LAV JyEG9SLQlh21LrSSoJ+zrWaMJjt4UpGul3FXE5f3viIkfIzShcI4lkStWK1hoU80 1RIroKU1JyGta/0kn8VfMvGzhEL72hCs9ivlMhXmvw1avFjQeIIJuR9QY2pQFe+a BaO5ds+bidBJtjGGBfwr182ev8ZGb0vscR2kTw7I8NrOHkq4FiXAOKCesRIN419n aeHET10Lsg4fBbclcPQ3 =7Bs7 -----END PGP SIGNATURE----- --=-+gJ+VlGcbDOhan6s7ILw-- --===============0446397706== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0446397706==--