* [PATCH 2/4] xf86drmMode: Unconditionally clear ioctl structs
2015-02-11 11:42 [PATCH 1/4] intel: Unconditionally clear ioctl structs Daniel Vetter
@ 2015-02-11 11:42 ` Daniel Vetter
2015-02-11 19:10 ` Ian Romanick
2015-02-11 11:42 ` [PATCH 3/4] drm: use drmIoctl everywhere Daniel Vetter
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 11:42 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter, Daniel Vetter
We really have to do this to avoid surprises when extending the ABI
later on. Especially when growing the structures.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
xf86drmMode.c | 55 ++++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/xf86drmMode.c b/xf86drmMode.c
index e3e599bdc39d..9ea8fe721842 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -61,7 +61,7 @@
#define VG(x)
#endif
-#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
+#define memclear(s) memset(&s, 0, sizeof(s))
#define U642VOID(x) ((void *)(unsigned long)(x))
#define VOID2U64(x) ((uint64_t)(unsigned long)(x))
@@ -164,7 +164,7 @@ drmModeResPtr drmModeGetResources(int fd)
drmModeResPtr r = 0;
retry:
- memset(&res, 0, sizeof(struct drm_mode_card_res));
+ memclear(res);
if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
return 0;
@@ -259,7 +259,7 @@ int drmModeAddFB(int fd, uint32_t width, uint32_t height, uint8_t depth,
struct drm_mode_fb_cmd f;
int ret;
- VG_CLEAR(f);
+ memclear(f);
f.width = width;
f.height = height;
f.pitch = pitch;
@@ -282,6 +282,7 @@ int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
struct drm_mode_fb_cmd2 f;
int ret;
+ memclear(f);
f.width = width;
f.height = height;
f.pixel_format = pixel_format;
@@ -300,8 +301,6 @@ int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
int drmModeRmFB(int fd, uint32_t bufferId)
{
return DRM_IOCTL(fd, DRM_IOCTL_MODE_RMFB, &bufferId);
-
-
}
drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
@@ -309,6 +308,7 @@ drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
struct drm_mode_fb_cmd info;
drmModeFBPtr r;
+ memclear(info);
info.fb_id = buf;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &info))
@@ -331,8 +331,9 @@ drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
int drmModeDirtyFB(int fd, uint32_t bufferId,
drmModeClipPtr clips, uint32_t num_clips)
{
- struct drm_mode_fb_dirty_cmd dirty = { 0 };
+ struct drm_mode_fb_dirty_cmd dirty;
+ memclear(dirty);
dirty.fb_id = bufferId;
dirty.clips_ptr = VOID2U64(clips);
dirty.num_clips = num_clips;
@@ -350,7 +351,7 @@ drmModeCrtcPtr drmModeGetCrtc(int fd, uint32_t crtcId)
struct drm_mode_crtc crtc;
drmModeCrtcPtr r;
- VG_CLEAR(crtc);
+ memclear(crtc);
crtc.crtc_id = crtcId;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc))
@@ -384,7 +385,7 @@ int drmModeSetCrtc(int fd, uint32_t crtcId, uint32_t bufferId,
{
struct drm_mode_crtc crtc;
- VG_CLEAR(crtc);
+ memclear(crtc);
crtc.x = x;
crtc.y = y;
crtc.crtc_id = crtcId;
@@ -394,8 +395,7 @@ int drmModeSetCrtc(int fd, uint32_t crtcId, uint32_t bufferId,
if (mode) {
memcpy(&crtc.mode, mode, sizeof(struct drm_mode_modeinfo));
crtc.mode_valid = 1;
- } else
- crtc.mode_valid = 0;
+ }
return DRM_IOCTL(fd, DRM_IOCTL_MODE_SETCRTC, &crtc);
}
@@ -408,6 +408,7 @@ int drmModeSetCursor(int fd, uint32_t crtcId, uint32_t bo_handle, uint32_t width
{
struct drm_mode_cursor arg;
+ memclear(arg);
arg.flags = DRM_MODE_CURSOR_BO;
arg.crtc_id = crtcId;
arg.width = width;
@@ -421,6 +422,7 @@ int drmModeSetCursor2(int fd, uint32_t crtcId, uint32_t bo_handle, uint32_t widt
{
struct drm_mode_cursor2 arg;
+ memclear(arg);
arg.flags = DRM_MODE_CURSOR_BO;
arg.crtc_id = crtcId;
arg.width = width;
@@ -436,6 +438,7 @@ int drmModeMoveCursor(int fd, uint32_t crtcId, int x, int y)
{
struct drm_mode_cursor arg;
+ memclear(arg);
arg.flags = DRM_MODE_CURSOR_MOVE;
arg.crtc_id = crtcId;
arg.x = x;
@@ -452,11 +455,8 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
struct drm_mode_get_encoder enc;
drmModeEncoderPtr r = NULL;
+ memclear(enc);
enc.encoder_id = encoder_id;
- enc.crtc_id = 0;
- enc.encoder_type = 0;
- enc.possible_crtcs = 0;
- enc.possible_clones = 0;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETENCODER, &enc))
return 0;
@@ -483,7 +483,7 @@ drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
drmModeConnectorPtr r = NULL;
retry:
- memset(&conn, 0, sizeof(struct drm_mode_get_connector));
+ memclear(conn);
conn.connector_id = connector_id;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
@@ -576,6 +576,7 @@ int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_inf
{
struct drm_mode_mode_cmd res;
+ memclear(res);
memcpy(&res.mode, mode_info, sizeof(struct drm_mode_modeinfo));
res.connector_id = connector_id;
@@ -586,6 +587,7 @@ int drmModeDetachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_inf
{
struct drm_mode_mode_cmd res;
+ memclear(res);
memcpy(&res.mode, mode_info, sizeof(struct drm_mode_modeinfo));
res.connector_id = connector_id;
@@ -598,13 +600,8 @@ drmModePropertyPtr drmModeGetProperty(int fd, uint32_t property_id)
struct drm_mode_get_property prop;
drmModePropertyPtr r;
- VG_CLEAR(prop);
+ memclear(prop);
prop.prop_id = property_id;
- prop.count_enum_blobs = 0;
- prop.count_values = 0;
- prop.flags = 0;
- prop.enum_blob_ptr = 0;
- prop.values_ptr = 0;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop))
return 0;
@@ -667,8 +664,7 @@ drmModePropertyBlobPtr drmModeGetPropertyBlob(int fd, uint32_t blob_id)
struct drm_mode_get_blob blob;
drmModePropertyBlobPtr r;
- blob.length = 0;
- blob.data = 0;
+ memclear(blob);
blob.blob_id = blob_id;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
@@ -708,6 +704,7 @@ int drmModeConnectorSetProperty(int fd, uint32_t connector_id, uint32_t property
{
struct drm_mode_connector_set_property osp;
+ memclear(osp);
osp.connector_id = connector_id;
osp.prop_id = property_id;
osp.value = value;
@@ -818,6 +815,7 @@ int drmModeCrtcGetGamma(int fd, uint32_t crtc_id, uint32_t size,
{
struct drm_mode_crtc_lut l;
+ memclear(l);
l.crtc_id = crtc_id;
l.gamma_size = size;
l.red = VOID2U64(red);
@@ -832,6 +830,7 @@ int drmModeCrtcSetGamma(int fd, uint32_t crtc_id, uint32_t size,
{
struct drm_mode_crtc_lut l;
+ memclear(l);
l.crtc_id = crtc_id;
l.gamma_size = size;
l.red = VOID2U64(red);
@@ -897,11 +896,11 @@ int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t fb_id,
{
struct drm_mode_crtc_page_flip flip;
+ memclear(flip);
flip.fb_id = fb_id;
flip.crtc_id = crtc_id;
flip.user_data = VOID2U64(user_data);
flip.flags = flags;
- flip.reserved = 0;
return DRM_IOCTL(fd, DRM_IOCTL_MODE_PAGE_FLIP, &flip);
}
@@ -916,6 +915,7 @@ int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
{
struct drm_mode_set_plane s;
+ memclear(s);
s.plane_id = plane_id;
s.crtc_id = crtc_id;
s.fb_id = fb_id;
@@ -939,7 +939,7 @@ drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
drmModePlanePtr r = 0;
retry:
- memset(&ovr, 0, sizeof(struct drm_mode_get_plane));
+ memclear(ovr);
ovr.plane_id = plane_id;
if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
return 0;
@@ -999,7 +999,7 @@ drmModePlaneResPtr drmModeGetPlaneResources(int fd)
drmModePlaneResPtr r = 0;
retry:
- memset(&res, 0, sizeof(struct drm_mode_get_plane_res));
+ memclear(res);
if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANERESOURCES, &res))
return 0;
@@ -1056,7 +1056,7 @@ drmModeObjectPropertiesPtr drmModeObjectGetProperties(int fd,
uint32_t count;
retry:
- memset(&properties, 0, sizeof(struct drm_mode_obj_get_properties));
+ memclear(properties);
properties.obj_id = object_id;
properties.obj_type = object_type;
@@ -1122,6 +1122,7 @@ int drmModeObjectSetProperty(int fd, uint32_t object_id, uint32_t object_type,
{
struct drm_mode_obj_set_property prop;
+ memclear(prop);
prop.value = value;
prop.prop_id = property_id;
prop.obj_id = object_id;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] xf86drmMode: Unconditionally clear ioctl structs
2015-02-11 11:42 ` [PATCH 2/4] xf86drmMode: " Daniel Vetter
@ 2015-02-11 19:10 ` Ian Romanick
0 siblings, 0 replies; 13+ messages in thread
From: Ian Romanick @ 2015-02-11 19:10 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
This patch is
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
On 02/11/2015 03:42 AM, Daniel Vetter wrote:
> We really have to do this to avoid surprises when extending the ABI
> later on. Especially when growing the structures.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> xf86drmMode.c | 55 ++++++++++++++++++++++++++++---------------------------
> 1 file changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index e3e599bdc39d..9ea8fe721842 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -61,7 +61,7 @@
> #define VG(x)
> #endif
>
> -#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
> +#define memclear(s) memset(&s, 0, sizeof(s))
>
> #define U642VOID(x) ((void *)(unsigned long)(x))
> #define VOID2U64(x) ((uint64_t)(unsigned long)(x))
> @@ -164,7 +164,7 @@ drmModeResPtr drmModeGetResources(int fd)
> drmModeResPtr r = 0;
>
> retry:
> - memset(&res, 0, sizeof(struct drm_mode_card_res));
> + memclear(res);
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
> return 0;
>
> @@ -259,7 +259,7 @@ int drmModeAddFB(int fd, uint32_t width, uint32_t height, uint8_t depth,
> struct drm_mode_fb_cmd f;
> int ret;
>
> - VG_CLEAR(f);
> + memclear(f);
> f.width = width;
> f.height = height;
> f.pitch = pitch;
> @@ -282,6 +282,7 @@ int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> struct drm_mode_fb_cmd2 f;
> int ret;
>
> + memclear(f);
> f.width = width;
> f.height = height;
> f.pixel_format = pixel_format;
> @@ -300,8 +301,6 @@ int drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> int drmModeRmFB(int fd, uint32_t bufferId)
> {
> return DRM_IOCTL(fd, DRM_IOCTL_MODE_RMFB, &bufferId);
> -
> -
> }
>
> drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
> @@ -309,6 +308,7 @@ drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
> struct drm_mode_fb_cmd info;
> drmModeFBPtr r;
>
> + memclear(info);
> info.fb_id = buf;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETFB, &info))
> @@ -331,8 +331,9 @@ drmModeFBPtr drmModeGetFB(int fd, uint32_t buf)
> int drmModeDirtyFB(int fd, uint32_t bufferId,
> drmModeClipPtr clips, uint32_t num_clips)
> {
> - struct drm_mode_fb_dirty_cmd dirty = { 0 };
> + struct drm_mode_fb_dirty_cmd dirty;
>
> + memclear(dirty);
> dirty.fb_id = bufferId;
> dirty.clips_ptr = VOID2U64(clips);
> dirty.num_clips = num_clips;
> @@ -350,7 +351,7 @@ drmModeCrtcPtr drmModeGetCrtc(int fd, uint32_t crtcId)
> struct drm_mode_crtc crtc;
> drmModeCrtcPtr r;
>
> - VG_CLEAR(crtc);
> + memclear(crtc);
> crtc.crtc_id = crtcId;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc))
> @@ -384,7 +385,7 @@ int drmModeSetCrtc(int fd, uint32_t crtcId, uint32_t bufferId,
> {
> struct drm_mode_crtc crtc;
>
> - VG_CLEAR(crtc);
> + memclear(crtc);
> crtc.x = x;
> crtc.y = y;
> crtc.crtc_id = crtcId;
> @@ -394,8 +395,7 @@ int drmModeSetCrtc(int fd, uint32_t crtcId, uint32_t bufferId,
> if (mode) {
> memcpy(&crtc.mode, mode, sizeof(struct drm_mode_modeinfo));
> crtc.mode_valid = 1;
> - } else
> - crtc.mode_valid = 0;
> + }
>
> return DRM_IOCTL(fd, DRM_IOCTL_MODE_SETCRTC, &crtc);
> }
> @@ -408,6 +408,7 @@ int drmModeSetCursor(int fd, uint32_t crtcId, uint32_t bo_handle, uint32_t width
> {
> struct drm_mode_cursor arg;
>
> + memclear(arg);
> arg.flags = DRM_MODE_CURSOR_BO;
> arg.crtc_id = crtcId;
> arg.width = width;
> @@ -421,6 +422,7 @@ int drmModeSetCursor2(int fd, uint32_t crtcId, uint32_t bo_handle, uint32_t widt
> {
> struct drm_mode_cursor2 arg;
>
> + memclear(arg);
> arg.flags = DRM_MODE_CURSOR_BO;
> arg.crtc_id = crtcId;
> arg.width = width;
> @@ -436,6 +438,7 @@ int drmModeMoveCursor(int fd, uint32_t crtcId, int x, int y)
> {
> struct drm_mode_cursor arg;
>
> + memclear(arg);
> arg.flags = DRM_MODE_CURSOR_MOVE;
> arg.crtc_id = crtcId;
> arg.x = x;
> @@ -452,11 +455,8 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
> struct drm_mode_get_encoder enc;
> drmModeEncoderPtr r = NULL;
>
> + memclear(enc);
> enc.encoder_id = encoder_id;
> - enc.crtc_id = 0;
> - enc.encoder_type = 0;
> - enc.possible_crtcs = 0;
> - enc.possible_clones = 0;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETENCODER, &enc))
> return 0;
> @@ -483,7 +483,7 @@ drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
> drmModeConnectorPtr r = NULL;
>
> retry:
> - memset(&conn, 0, sizeof(struct drm_mode_get_connector));
> + memclear(conn);
> conn.connector_id = connector_id;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
> @@ -576,6 +576,7 @@ int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_inf
> {
> struct drm_mode_mode_cmd res;
>
> + memclear(res);
> memcpy(&res.mode, mode_info, sizeof(struct drm_mode_modeinfo));
> res.connector_id = connector_id;
>
> @@ -586,6 +587,7 @@ int drmModeDetachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_inf
> {
> struct drm_mode_mode_cmd res;
>
> + memclear(res);
> memcpy(&res.mode, mode_info, sizeof(struct drm_mode_modeinfo));
> res.connector_id = connector_id;
>
> @@ -598,13 +600,8 @@ drmModePropertyPtr drmModeGetProperty(int fd, uint32_t property_id)
> struct drm_mode_get_property prop;
> drmModePropertyPtr r;
>
> - VG_CLEAR(prop);
> + memclear(prop);
> prop.prop_id = property_id;
> - prop.count_enum_blobs = 0;
> - prop.count_values = 0;
> - prop.flags = 0;
> - prop.enum_blob_ptr = 0;
> - prop.values_ptr = 0;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop))
> return 0;
> @@ -667,8 +664,7 @@ drmModePropertyBlobPtr drmModeGetPropertyBlob(int fd, uint32_t blob_id)
> struct drm_mode_get_blob blob;
> drmModePropertyBlobPtr r;
>
> - blob.length = 0;
> - blob.data = 0;
> + memclear(blob);
> blob.blob_id = blob_id;
>
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> @@ -708,6 +704,7 @@ int drmModeConnectorSetProperty(int fd, uint32_t connector_id, uint32_t property
> {
> struct drm_mode_connector_set_property osp;
>
> + memclear(osp);
> osp.connector_id = connector_id;
> osp.prop_id = property_id;
> osp.value = value;
> @@ -818,6 +815,7 @@ int drmModeCrtcGetGamma(int fd, uint32_t crtc_id, uint32_t size,
> {
> struct drm_mode_crtc_lut l;
>
> + memclear(l);
> l.crtc_id = crtc_id;
> l.gamma_size = size;
> l.red = VOID2U64(red);
> @@ -832,6 +830,7 @@ int drmModeCrtcSetGamma(int fd, uint32_t crtc_id, uint32_t size,
> {
> struct drm_mode_crtc_lut l;
>
> + memclear(l);
> l.crtc_id = crtc_id;
> l.gamma_size = size;
> l.red = VOID2U64(red);
> @@ -897,11 +896,11 @@ int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t fb_id,
> {
> struct drm_mode_crtc_page_flip flip;
>
> + memclear(flip);
> flip.fb_id = fb_id;
> flip.crtc_id = crtc_id;
> flip.user_data = VOID2U64(user_data);
> flip.flags = flags;
> - flip.reserved = 0;
>
> return DRM_IOCTL(fd, DRM_IOCTL_MODE_PAGE_FLIP, &flip);
> }
> @@ -916,6 +915,7 @@ int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
> {
> struct drm_mode_set_plane s;
>
> + memclear(s);
> s.plane_id = plane_id;
> s.crtc_id = crtc_id;
> s.fb_id = fb_id;
> @@ -939,7 +939,7 @@ drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
> drmModePlanePtr r = 0;
>
> retry:
> - memset(&ovr, 0, sizeof(struct drm_mode_get_plane));
> + memclear(ovr);
> ovr.plane_id = plane_id;
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
> return 0;
> @@ -999,7 +999,7 @@ drmModePlaneResPtr drmModeGetPlaneResources(int fd)
> drmModePlaneResPtr r = 0;
>
> retry:
> - memset(&res, 0, sizeof(struct drm_mode_get_plane_res));
> + memclear(res);
> if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANERESOURCES, &res))
> return 0;
>
> @@ -1056,7 +1056,7 @@ drmModeObjectPropertiesPtr drmModeObjectGetProperties(int fd,
> uint32_t count;
>
> retry:
> - memset(&properties, 0, sizeof(struct drm_mode_obj_get_properties));
> + memclear(properties);
> properties.obj_id = object_id;
> properties.obj_type = object_type;
>
> @@ -1122,6 +1122,7 @@ int drmModeObjectSetProperty(int fd, uint32_t object_id, uint32_t object_type,
> {
> struct drm_mode_obj_set_property prop;
>
> + memclear(prop);
> prop.value = value;
> prop.prop_id = property_id;
> prop.obj_id = object_id;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] drm: use drmIoctl everywhere
2015-02-11 11:42 [PATCH 1/4] intel: Unconditionally clear ioctl structs Daniel Vetter
2015-02-11 11:42 ` [PATCH 2/4] xf86drmMode: " Daniel Vetter
@ 2015-02-11 11:42 ` Daniel Vetter
2015-02-11 12:21 ` Rob Clark
2015-02-11 13:13 ` Emil Velikov
2015-02-11 11:42 ` [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs Daniel Vetter
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 11:42 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter, Daniel Vetter
Well just core drm. All the other callers in there that still use
direct calls to ioctl have some custom retry logic already, so should
be good already.
freedreno/kgsl ahas all the other bare ioctl calls, dunnot what to do
about that.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
xf86drm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index fb673b58afb3..263d6835c29a 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2543,12 +2543,12 @@ void drmCloseOnce(int fd)
int drmSetMaster(int fd)
{
- return ioctl(fd, DRM_IOCTL_SET_MASTER, 0);
+ return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
}
int drmDropMaster(int fd)
{
- return ioctl(fd, DRM_IOCTL_DROP_MASTER, 0);
+ return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
}
char *drmGetDeviceNameFromFd(int fd)
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] drm: use drmIoctl everywhere
2015-02-11 11:42 ` [PATCH 3/4] drm: use drmIoctl everywhere Daniel Vetter
@ 2015-02-11 12:21 ` Rob Clark
2015-02-11 13:13 ` Emil Velikov
1 sibling, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-02-11 12:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Wed, Feb 11, 2015 at 6:42 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Well just core drm. All the other callers in there that still use
> direct calls to ioctl have some custom retry logic already, so should
> be good already.
>
> freedreno/kgsl ahas all the other bare ioctl calls, dunnot what to do
> about that.
fwiw, the freedreno/kgsl bare ioctls are actually against the non-drm
device file.. freedreno/msm is the stuff for the upstream drm/msm
driver.
background: downstream android kernel w/ kgsl kernel driver has this
shim drm driver which is really just for getting flink names + non drm
device for everything else. It's kinda bonghits, really just exists
to give folks stuck w/ android drivers a fighting chance. And due to
different versions of kgsl floating around on the many different
android kernel branches, odds are still pretty grim (which is why kgsl
backend is disabled by default)
I guess technically they could still use drmIoctl() but it seemed a bit strange
BR,
-R
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> xf86drm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index fb673b58afb3..263d6835c29a 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2543,12 +2543,12 @@ void drmCloseOnce(int fd)
>
> int drmSetMaster(int fd)
> {
> - return ioctl(fd, DRM_IOCTL_SET_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
> }
>
> int drmDropMaster(int fd)
> {
> - return ioctl(fd, DRM_IOCTL_DROP_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
> }
>
> char *drmGetDeviceNameFromFd(int fd)
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] drm: use drmIoctl everywhere
2015-02-11 11:42 ` [PATCH 3/4] drm: use drmIoctl everywhere Daniel Vetter
2015-02-11 12:21 ` Rob Clark
@ 2015-02-11 13:13 ` Emil Velikov
2015-02-11 14:34 ` Daniel Vetter
1 sibling, 1 reply; 13+ messages in thread
From: Emil Velikov @ 2015-02-11 13:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On 11 February 2015 at 11:42, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Well just core drm. All the other callers in there that still use
> direct calls to ioctl have some custom retry logic already, so should
> be good already.
>
Afaics intel/intel_bufmgr_gem.c has one instance, plus most of the tests.
> freedreno/kgsl ahas all the other bare ioctl calls, dunnot what to do
> about that.
>
s/ahas/has/ if you choose to keep this note.
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] drm: use drmIoctl everywhere
2015-02-11 13:13 ` Emil Velikov
@ 2015-02-11 14:34 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 14:34 UTC (permalink / raw)
To: Emil Velikov
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Feb 11, 2015 at 01:13:26PM +0000, Emil Velikov wrote:
> On 11 February 2015 at 11:42, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Well just core drm. All the other callers in there that still use
> > direct calls to ioctl have some custom retry logic already, so should
> > be good already.
> >
> Afaics intel/intel_bufmgr_gem.c has one instance, plus most of the tests.
That one is in set_tiling and because of a very peculiar retry semantics
of that specific ioctl. Don't ask ;-)
>
> > freedreno/kgsl ahas all the other bare ioctl calls, dunnot what to do
> > about that.
> >
> s/ahas/has/ if you choose to keep this note.
Already reworded since kgsl is the blob library and so not a drm thing.
Thanks for your review.
-Daniel
>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>
> -Emil
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs
2015-02-11 11:42 [PATCH 1/4] intel: Unconditionally clear ioctl structs Daniel Vetter
2015-02-11 11:42 ` [PATCH 2/4] xf86drmMode: " Daniel Vetter
2015-02-11 11:42 ` [PATCH 3/4] drm: use drmIoctl everywhere Daniel Vetter
@ 2015-02-11 11:42 ` Daniel Vetter
2015-02-11 13:20 ` Emil Velikov
2015-02-11 15:57 ` Jan Vesely
2015-02-11 11:56 ` [PATCH] tests: remove intel-specific tests Daniel Vetter
2015-02-11 19:09 ` [PATCH 1/4] intel: Unconditionally clear ioctl structs Ian Romanick
4 siblings, 2 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 11:42 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter, Daniel Vetter
We really have to do this to avoid surprises when extending the ABI
later on. Especially when growing the structures.
A bit overkill to update all the old legacy ioctl wrappers, but can't
hurt really either.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
xf86drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 77 insertions(+), 35 deletions(-)
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
+#define memclear(s) memset(&s, 0, sizeof(s))
+
static drmServerInfoPtr drm_server_info;
void drmSetServerInfo(drmServerInfoPtr info)
@@ -766,12 +768,7 @@ drmVersionPtr drmGetVersion(int fd)
drmVersionPtr retval;
drm_version_t *version = drmMalloc(sizeof(*version));
- version->name_len = 0;
- version->name = NULL;
- version->date_len = 0;
- version->date = NULL;
- version->desc_len = 0;
- version->desc = NULL;
+ memclear(version);
if (drmIoctl(fd, DRM_IOCTL_VERSION, version)) {
drmFreeKernelVersion(version);
@@ -839,9 +836,12 @@ drmVersionPtr drmGetLibVersion(int fd)
int drmGetCap(int fd, uint64_t capability, uint64_t *value)
{
- struct drm_get_cap cap = { capability, 0 };
+ struct drm_get_cap cap;
int ret;
+ memclear(cap);
+ cap.capability = capability;
+
ret = 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)
int drmSetClientCap(int fd, uint64_t capability, uint64_t value)
{
- struct drm_set_client_cap cap = { capability, value };
+ struct drm_set_client_cap cap;
+
+ memclear(cap);
+ cap.capability = capability;
+ cap.value = value;
return drmIoctl(fd, DRM_IOCTL_SET_CLIENT_CAP, &cap);
}
@@ -887,8 +891,7 @@ char *drmGetBusid(int fd)
{
drm_unique_t u;
- u.unique_len = 0;
- u.unique = NULL;
+ memclear(u);
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;
+ memclear(u);
u.unique = (char *)busid;
u.unique_len = strlen(busid);
@@ -930,6 +934,8 @@ int drmGetMagic(int fd, drm_magic_t * magic)
{
drm_auth_t auth;
+ memclear(auth);
+
*magic = 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;
+ memclear(auth);
auth.magic = 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;
+ memclear(map);
map.offset = offset;
map.size = size;
- map.handle = 0;
map.type = type;
map.flags = 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;
+ memclear(map);
map.handle = (void *)(uintptr_t)handle;
if(drmIoctl(fd, DRM_IOCTL_RM_MAP, &map))
@@ -1046,10 +1054,9 @@ int drmAddBufs(int fd, int count, int size, drmBufDescFlags flags,
{
drm_buf_desc_t request;
+ memclear(request);
request.count = count;
request.size = size;
- request.low_mark = 0;
- request.high_mark = 0;
request.flags = flags;
request.agp_start = agp_offset;
@@ -1063,8 +1070,7 @@ int drmMarkBufs(int fd, double low, double high)
drm_buf_info_t info;
int i;
- info.count = 0;
- info.list = NULL;
+ memclear(info);
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;
+ memclear(request);
request.count = count;
request.list = list;
if (drmIoctl(fd, DRM_IOCTL_FREE_BUFS, &request))
@@ -1202,8 +1209,7 @@ drmBufInfoPtr drmGetBufInfo(int fd)
drmBufInfoPtr retval;
int i;
- info.count = 0;
- info.list = NULL;
+ memclear(info);
if (drmIoctl(fd, DRM_IOCTL_INFO_BUFS, &info))
return NULL;
@@ -1253,9 +1259,7 @@ drmBufMapPtr drmMapBufs(int fd)
drmBufMapPtr retval;
int i;
- bufs.count = 0;
- bufs.list = NULL;
- bufs.virtual = NULL;
+ memclear(bufs);
if (drmIoctl(fd, DRM_IOCTL_MAP_BUFS, &bufs))
return NULL;
@@ -1371,6 +1375,7 @@ int drmGetLock(int fd, drm_context_t context, drmLockFlags flags)
{
drm_lock_t lock;
+ memclear(lock);
lock.context = context;
lock.flags = 0;
if (flags & DRM_LOCK_READY) lock.flags |= _DRM_LOCK_READY;
@@ -1401,8 +1406,8 @@ int drmUnlock(int fd, drm_context_t context)
{
drm_lock_t lock;
+ memclear(lock);
lock.context = context;
- lock.flags = 0;
return drmIoctl(fd, DRM_IOCTL_UNLOCK, &lock);
}
@@ -1413,8 +1418,7 @@ drm_context_t *drmGetReservedContextList(int fd, int *count)
drm_context_t * retval;
int i;
- res.count = 0;
- res.contexts = NULL;
+ memclear(res);
if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res))
return NULL;
@@ -1467,7 +1471,7 @@ int drmCreateContext(int fd, drm_context_t *handle)
{
drm_ctx_t ctx;
- ctx.flags = 0; /* Modified with functions below */
+ memclear(ctx);
if (drmIoctl(fd, DRM_IOCTL_ADD_CTX, &ctx))
return -errno;
*handle = ctx.handle;
@@ -1478,6 +1482,7 @@ int drmSwitchToContext(int fd, drm_context_t context)
{
drm_ctx_t ctx;
+ memclear(ctx);
ctx.handle = context;
if (drmIoctl(fd, DRM_IOCTL_SWITCH_CTX, &ctx))
return -errno;
@@ -1494,8 +1499,8 @@ int drmSetContextFlags(int fd, drm_context_t context, 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 threads.
*/
+ memclear(ctx);
ctx.handle = context;
- ctx.flags = 0;
if (flags & DRM_CONTEXT_PRESERVED)
ctx.flags |= _DRM_CONTEXT_PRESERVED;
if (flags & DRM_CONTEXT_2DONLY)
@@ -1510,6 +1515,7 @@ int drmGetContextFlags(int fd, drm_context_t context,
{
drm_ctx_t ctx;
+ memclear(ctx);
ctx.handle = context;
if (drmIoctl(fd, DRM_IOCTL_GET_CTX, &ctx))
return -errno;
@@ -1541,6 +1547,8 @@ int drmGetContextFlags(int fd, drm_context_t context,
int drmDestroyContext(int fd, drm_context_t handle)
{
drm_ctx_t ctx;
+
+ memclear(ctx);
ctx.handle = 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 = draw.handle;
@@ -1559,6 +1569,8 @@ int drmCreateDrawable(int fd, drm_drawable_t *handle)
int drmDestroyDrawable(int fd, drm_drawable_t handle)
{
drm_draw_t draw;
+
+ memclear(draw);
draw.handle = handle;
if (drmIoctl(fd, DRM_IOCTL_RM_DRAW, &draw))
return -errno;
@@ -1571,6 +1583,7 @@ int drmUpdateDrawableInfo(int fd, drm_drawable_t handle,
{
drm_update_draw_t update;
+ memclear(update);
update.handle = handle;
update.type = type;
update.num = num;
@@ -1636,6 +1649,7 @@ int drmAgpEnable(int fd, unsigned long mode)
{
drm_agp_mode_t m;
+ memclear(mode);
m.mode = mode;
if (drmIoctl(fd, DRM_IOCTL_AGP_ENABLE, &m))
return -errno;
@@ -1664,9 +1678,9 @@ int drmAgpAlloc(int fd, unsigned long size, unsigned long type,
{
drm_agp_buffer_t b;
+ memclear(b);
*handle = DRM_AGP_NO_HANDLE;
b.size = size;
- b.handle = 0;
b.type = 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;
- b.size = 0;
+ memclear(b);
b.handle = handle;
if (drmIoctl(fd, DRM_IOCTL_AGP_FREE, &b))
return -errno;
@@ -1718,6 +1732,7 @@ int drmAgpBind(int fd, drm_handle_t handle, unsigned long offset)
{
drm_agp_binding_t b;
+ memclear(b);
b.handle = handle;
b.offset = 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;
+ memclear(b);
b.handle = handle;
- b.offset = 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;
+ 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;
+ 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;
+ 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;
+ 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;
+ 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;
+ 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;
+ 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;
+ 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;
+ 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 size, drm_handle_t *handle)
{
drm_scatter_gather_t sg;
+ memclear(sg);
+
*handle = 0;
sg.size = size;
- sg.handle = 0;
if (drmIoctl(fd, DRM_IOCTL_SG_ALLOC, &sg))
return -errno;
*handle = sg.handle;
@@ -1955,7 +1989,7 @@ int drmScatterGatherFree(int fd, drm_handle_t handle)
{
drm_scatter_gather_t sg;
- sg.size = 0;
+ memclear(sg);
sg.handle = 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;
+ memclear(ctl);
ctl.func = DRM_INST_HANDLER;
ctl.irq = irq;
if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl))
@@ -2069,6 +2104,7 @@ int drmCtlUninstHandler(int fd)
{
drm_control_t ctl;
+ memclear(ctl);
ctl.func = DRM_UNINST_HANDLER;
ctl.irq = 0;
if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl))
@@ -2080,8 +2116,8 @@ int drmFinish(int fd, int context, drmLockFlags flags)
{
drm_lock_t lock;
+ memclear(lock);
lock.context = context;
- lock.flags = 0;
if (flags & DRM_LOCK_READY) lock.flags |= _DRM_LOCK_READY;
if (flags & DRM_LOCK_QUIESCENT) lock.flags |= _DRM_LOCK_QUIESCENT;
if (flags & DRM_LOCK_FLUSH) lock.flags |= _DRM_LOCK_FLUSH;
@@ -2111,6 +2147,7 @@ int drmGetInterruptFromBusID(int fd, int busnum, int devnum, int funcnum)
{
drm_irq_busid_t p;
+ memclear(p);
p.busnum = busnum;
p.devnum = devnum;
p.funcnum = funcnum;
@@ -2153,6 +2190,7 @@ int drmAddContextPrivateMapping(int fd, drm_context_t ctx_id,
{
drm_ctx_priv_map_t map;
+ memclear(map);
map.ctx_id = ctx_id;
map.handle = (void *)(uintptr_t)handle;
@@ -2166,6 +2204,7 @@ int drmGetContextPrivateMapping(int fd, drm_context_t ctx_id,
{
drm_ctx_priv_map_t map;
+ memclear(map);
map.ctx_id = ctx_id;
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;
+ memclear(map);
map.offset = idx;
if (drmIoctl(fd, DRM_IOCTL_GET_MAP, &map))
return -errno;
@@ -2199,6 +2239,7 @@ int drmGetClient(int fd, int idx, int *auth, int *pid, int *uid,
{
drm_client_t client;
+ memclear(client);
client.idx = 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;
+ memclear(s);
if (drmIoctl(fd, DRM_IOCTL_GET_STATS, &s))
return -errno;
@@ -2352,6 +2394,7 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *version)
int retcode = 0;
drm_set_version_t sv;
+ memclear(sv);
sv.drm_di_major = version->drm_di_major;
sv.drm_di_minor = version->drm_di_minor;
sv.drm_dd_major = version->drm_dd_major;
@@ -2383,12 +2426,11 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *version)
*/
int drmCommandNone(int fd, unsigned long drmCommandIndex)
{
- void *data = NULL; /* dummy */
unsigned long request;
request = DRM_IO( DRM_COMMAND_BASE + drmCommandIndex);
- if (drmIoctl(fd, request, data)) {
+ if (drmIoctl(fd, request, NULL)) {
return -errno;
}
return 0;
@@ -2543,12 +2585,12 @@ void drmCloseOnce(int fd)
int drmSetMaster(int fd)
{
- return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
+ return drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL);
}
int drmDropMaster(int fd)
{
- return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
+ return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
}
char *drmGetDeviceNameFromFd(int fd)
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs
2015-02-11 11:42 ` [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs Daniel Vetter
@ 2015-02-11 13:20 ` Emil Velikov
2015-02-11 15:57 ` Jan Vesely
1 sibling, 0 replies; 13+ messages in thread
From: Emil Velikov @ 2015-02-11 13:20 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On 11 February 2015 at 11:42, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We really have to do this to avoid surprises when extending the ABI
> later on. Especially when growing the structures.
>
> A bit overkill to update all the old legacy ioctl wrappers, but can't
> hurt really either.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> xf86drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 77 insertions(+), 35 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 263d6835c29a..a2e24eb5f76c 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
[snip]
> @@ -2383,12 +2426,11 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *version)
> */
> int drmCommandNone(int fd, unsigned long drmCommandIndex)
> {
> - void *data = NULL; /* dummy */
> unsigned long request;
>
> request = DRM_IO( DRM_COMMAND_BASE + drmCommandIndex);
>
> - if (drmIoctl(fd, request, data)) {
> + if (drmIoctl(fd, request, NULL)) {
> return -errno;
> }
> return 0;
> @@ -2543,12 +2585,12 @@ void drmCloseOnce(int fd)
>
> int drmSetMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL);
> }
>
> int drmDropMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
> }
>
These hunks are not strictly related, and maybe we can split it out.
Either way the series looks good.
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs
2015-02-11 11:42 ` [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs Daniel Vetter
2015-02-11 13:20 ` Emil Velikov
@ 2015-02-11 15:57 ` Jan Vesely
2015-02-11 16:21 ` Daniel Vetter
1 sibling, 1 reply; 13+ messages in thread
From: Jan Vesely @ 2015-02-11 15:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
[-- Attachment #1.1: Type: text/plain, Size: 15859 bytes --]
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.
>
> A bit overkill to update all the old legacy ioctl wrappers, but can't
> hurt really either.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> xf86drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 77 insertions(+), 35 deletions(-)
>
> 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
>
> +#define memclear(s) memset(&s, 0, sizeof(s))
> +
> static drmServerInfoPtr drm_server_info;
>
> void drmSetServerInfo(drmServerInfoPtr info)
> @@ -766,12 +768,7 @@ drmVersionPtr drmGetVersion(int fd)
> drmVersionPtr retval;
> drm_version_t *version = drmMalloc(sizeof(*version));
>
> - version->name_len = 0;
> - version->name = NULL;
> - version->date_len = 0;
> - version->date = NULL;
> - version->desc_len = 0;
> - version->desc = NULL;
> + memclear(version);
I think this should be memclear(*version).
Otherwise it clears the pointer not the structure.
>
> if (drmIoctl(fd, DRM_IOCTL_VERSION, version)) {
> drmFreeKernelVersion(version);
> @@ -839,9 +836,12 @@ drmVersionPtr drmGetLibVersion(int fd)
>
> int drmGetCap(int fd, uint64_t capability, uint64_t *value)
> {
> - struct drm_get_cap cap = { capability, 0 };
> + struct drm_get_cap cap;
> int ret;
>
> + memclear(cap);
> + cap.capability = capability;
> +
> ret = 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)
>
> int drmSetClientCap(int fd, uint64_t capability, uint64_t value)
> {
> - struct drm_set_client_cap cap = { capability, value };
> + struct drm_set_client_cap cap;
> +
> + memclear(cap);
> + cap.capability = capability;
> + cap.value = value;
>
> return drmIoctl(fd, DRM_IOCTL_SET_CLIENT_CAP, &cap);
> }
> @@ -887,8 +891,7 @@ char *drmGetBusid(int fd)
> {
> drm_unique_t u;
>
> - u.unique_len = 0;
> - u.unique = NULL;
> + memclear(u);
>
> 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;
>
> + memclear(u);
> u.unique = (char *)busid;
> u.unique_len = strlen(busid);
>
> @@ -930,6 +934,8 @@ int drmGetMagic(int fd, drm_magic_t * magic)
> {
> drm_auth_t auth;
>
> + memclear(auth);
> +
> *magic = 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;
>
> + memclear(auth);
> auth.magic = 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;
>
> + memclear(map);
> map.offset = offset;
> map.size = size;
> - map.handle = 0;
> map.type = type;
> map.flags = 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;
>
> + memclear(map);
> map.handle = (void *)(uintptr_t)handle;
>
> if(drmIoctl(fd, DRM_IOCTL_RM_MAP, &map))
> @@ -1046,10 +1054,9 @@ int drmAddBufs(int fd, int count, int size, drmBufDescFlags flags,
> {
> drm_buf_desc_t request;
>
> + memclear(request);
> request.count = count;
> request.size = size;
> - request.low_mark = 0;
> - request.high_mark = 0;
> request.flags = flags;
> request.agp_start = agp_offset;
>
> @@ -1063,8 +1070,7 @@ int drmMarkBufs(int fd, double low, double high)
> drm_buf_info_t info;
> int i;
>
> - info.count = 0;
> - info.list = NULL;
> + memclear(info);
>
> 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;
>
> + memclear(request);
> request.count = count;
> request.list = list;
> if (drmIoctl(fd, DRM_IOCTL_FREE_BUFS, &request))
> @@ -1202,8 +1209,7 @@ drmBufInfoPtr drmGetBufInfo(int fd)
> drmBufInfoPtr retval;
> int i;
>
> - info.count = 0;
> - info.list = NULL;
> + memclear(info);
>
> if (drmIoctl(fd, DRM_IOCTL_INFO_BUFS, &info))
> return NULL;
> @@ -1253,9 +1259,7 @@ drmBufMapPtr drmMapBufs(int fd)
> drmBufMapPtr retval;
> int i;
>
> - bufs.count = 0;
> - bufs.list = NULL;
> - bufs.virtual = NULL;
> + memclear(bufs);
> if (drmIoctl(fd, DRM_IOCTL_MAP_BUFS, &bufs))
> return NULL;
>
> @@ -1371,6 +1375,7 @@ int drmGetLock(int fd, drm_context_t context, drmLockFlags flags)
> {
> drm_lock_t lock;
>
> + memclear(lock);
> lock.context = context;
> lock.flags = 0;
> if (flags & DRM_LOCK_READY) lock.flags |= _DRM_LOCK_READY;
> @@ -1401,8 +1406,8 @@ int drmUnlock(int fd, drm_context_t context)
> {
> drm_lock_t lock;
>
> + memclear(lock);
> lock.context = context;
> - lock.flags = 0;
> return drmIoctl(fd, DRM_IOCTL_UNLOCK, &lock);
> }
>
> @@ -1413,8 +1418,7 @@ drm_context_t *drmGetReservedContextList(int fd, int *count)
> drm_context_t * retval;
> int i;
>
> - res.count = 0;
> - res.contexts = NULL;
> + memclear(res);
> if (drmIoctl(fd, DRM_IOCTL_RES_CTX, &res))
> return NULL;
>
> @@ -1467,7 +1471,7 @@ int drmCreateContext(int fd, drm_context_t *handle)
> {
> drm_ctx_t ctx;
>
> - ctx.flags = 0; /* Modified with functions below */
> + memclear(ctx);
> if (drmIoctl(fd, DRM_IOCTL_ADD_CTX, &ctx))
> return -errno;
> *handle = ctx.handle;
> @@ -1478,6 +1482,7 @@ int drmSwitchToContext(int fd, drm_context_t context)
> {
> drm_ctx_t ctx;
>
> + memclear(ctx);
> ctx.handle = context;
> if (drmIoctl(fd, DRM_IOCTL_SWITCH_CTX, &ctx))
> return -errno;
> @@ -1494,8 +1499,8 @@ int drmSetContextFlags(int fd, drm_context_t context, 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 threads.
> */
> + memclear(ctx);
> ctx.handle = context;
> - ctx.flags = 0;
> if (flags & DRM_CONTEXT_PRESERVED)
> ctx.flags |= _DRM_CONTEXT_PRESERVED;
> if (flags & DRM_CONTEXT_2DONLY)
> @@ -1510,6 +1515,7 @@ int drmGetContextFlags(int fd, drm_context_t context,
> {
> drm_ctx_t ctx;
>
> + memclear(ctx);
> ctx.handle = context;
> if (drmIoctl(fd, DRM_IOCTL_GET_CTX, &ctx))
> return -errno;
> @@ -1541,6 +1547,8 @@ int drmGetContextFlags(int fd, drm_context_t context,
> int drmDestroyContext(int fd, drm_context_t handle)
> {
> drm_ctx_t ctx;
> +
> + memclear(ctx);
> ctx.handle = 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 = draw.handle;
> @@ -1559,6 +1569,8 @@ int drmCreateDrawable(int fd, drm_drawable_t *handle)
> int drmDestroyDrawable(int fd, drm_drawable_t handle)
> {
> drm_draw_t draw;
> +
> + memclear(draw);
> draw.handle = handle;
> if (drmIoctl(fd, DRM_IOCTL_RM_DRAW, &draw))
> return -errno;
> @@ -1571,6 +1583,7 @@ int drmUpdateDrawableInfo(int fd, drm_drawable_t handle,
> {
> drm_update_draw_t update;
>
> + memclear(update);
> update.handle = handle;
> update.type = type;
> update.num = num;
> @@ -1636,6 +1649,7 @@ int drmAgpEnable(int fd, unsigned long mode)
> {
> drm_agp_mode_t m;
>
> + memclear(mode);
> m.mode = mode;
> if (drmIoctl(fd, DRM_IOCTL_AGP_ENABLE, &m))
> return -errno;
> @@ -1664,9 +1678,9 @@ int drmAgpAlloc(int fd, unsigned long size, unsigned long type,
> {
> drm_agp_buffer_t b;
>
> + memclear(b);
> *handle = DRM_AGP_NO_HANDLE;
> b.size = size;
> - b.handle = 0;
> b.type = 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;
>
> - b.size = 0;
> + memclear(b);
> b.handle = handle;
> if (drmIoctl(fd, DRM_IOCTL_AGP_FREE, &b))
> return -errno;
> @@ -1718,6 +1732,7 @@ int drmAgpBind(int fd, drm_handle_t handle, unsigned long offset)
> {
> drm_agp_binding_t b;
>
> + memclear(b);
> b.handle = handle;
> b.offset = 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;
>
> + memclear(b);
> b.handle = handle;
> - b.offset = 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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;
>
> + 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 size, drm_handle_t *handle)
> {
> drm_scatter_gather_t sg;
>
> + memclear(sg);
> +
> *handle = 0;
> sg.size = size;
> - sg.handle = 0;
> if (drmIoctl(fd, DRM_IOCTL_SG_ALLOC, &sg))
> return -errno;
> *handle = sg.handle;
> @@ -1955,7 +1989,7 @@ int drmScatterGatherFree(int fd, drm_handle_t handle)
> {
> drm_scatter_gather_t sg;
>
> - sg.size = 0;
> + memclear(sg);
> sg.handle = 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;
>
> + memclear(ctl);
> ctl.func = DRM_INST_HANDLER;
> ctl.irq = irq;
> if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl))
> @@ -2069,6 +2104,7 @@ int drmCtlUninstHandler(int fd)
> {
> drm_control_t ctl;
>
> + memclear(ctl);
> ctl.func = DRM_UNINST_HANDLER;
> ctl.irq = 0;
> if (drmIoctl(fd, DRM_IOCTL_CONTROL, &ctl))
> @@ -2080,8 +2116,8 @@ int drmFinish(int fd, int context, drmLockFlags flags)
> {
> drm_lock_t lock;
>
> + memclear(lock);
> lock.context = context;
> - lock.flags = 0;
> if (flags & DRM_LOCK_READY) lock.flags |= _DRM_LOCK_READY;
> if (flags & DRM_LOCK_QUIESCENT) lock.flags |= _DRM_LOCK_QUIESCENT;
> if (flags & DRM_LOCK_FLUSH) lock.flags |= _DRM_LOCK_FLUSH;
> @@ -2111,6 +2147,7 @@ int drmGetInterruptFromBusID(int fd, int busnum, int devnum, int funcnum)
> {
> drm_irq_busid_t p;
>
> + memclear(p);
> p.busnum = busnum;
> p.devnum = devnum;
> p.funcnum = funcnum;
> @@ -2153,6 +2190,7 @@ int drmAddContextPrivateMapping(int fd, drm_context_t ctx_id,
> {
> drm_ctx_priv_map_t map;
>
> + memclear(map);
> map.ctx_id = ctx_id;
> map.handle = (void *)(uintptr_t)handle;
>
> @@ -2166,6 +2204,7 @@ int drmGetContextPrivateMapping(int fd, drm_context_t ctx_id,
> {
> drm_ctx_priv_map_t map;
>
> + memclear(map);
> map.ctx_id = ctx_id;
>
> 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;
>
> + memclear(map);
> map.offset = idx;
> if (drmIoctl(fd, DRM_IOCTL_GET_MAP, &map))
> return -errno;
> @@ -2199,6 +2239,7 @@ int drmGetClient(int fd, int idx, int *auth, int *pid, int *uid,
> {
> drm_client_t client;
>
> + memclear(client);
> client.idx = 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;
>
> + memclear(s);
> if (drmIoctl(fd, DRM_IOCTL_GET_STATS, &s))
> return -errno;
>
> @@ -2352,6 +2394,7 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *version)
> int retcode = 0;
> drm_set_version_t sv;
>
> + memclear(sv);
> sv.drm_di_major = version->drm_di_major;
> sv.drm_di_minor = version->drm_di_minor;
> sv.drm_dd_major = version->drm_dd_major;
> @@ -2383,12 +2426,11 @@ int drmSetInterfaceVersion(int fd, drmSetVersion *version)
> */
> int drmCommandNone(int fd, unsigned long drmCommandIndex)
> {
> - void *data = NULL; /* dummy */
> unsigned long request;
>
> request = DRM_IO( DRM_COMMAND_BASE + drmCommandIndex);
>
> - if (drmIoctl(fd, request, data)) {
> + if (drmIoctl(fd, request, NULL)) {
> return -errno;
> }
> return 0;
> @@ -2543,12 +2585,12 @@ void drmCloseOnce(int fd)
>
> int drmSetMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_SET_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL);
> }
>
> int drmDropMaster(int fd)
> {
> - return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, 0);
> + return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
> }
>
> char *drmGetDeviceNameFromFd(int fd)
--
Jan Vesely <jan.vesely@rutgers.edu>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs
2015-02-11 15:57 ` Jan Vesely
@ 2015-02-11 16:21 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 16:21 UTC (permalink / raw)
To: Jan Vesely
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
Daniel Vetter
On Wed, Feb 11, 2015 at 10:57:08AM -0500, Jan Vesely wrote:
> 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.
> >
> > A bit overkill to update all the old legacy ioctl wrappers, but can't
> > hurt really either.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > xf86drm.c | 112 ++++++++++++++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 77 insertions(+), 35 deletions(-)
> >
> > 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
> >
> > +#define memclear(s) memset(&s, 0, sizeof(s))
> > +
> > static drmServerInfoPtr drm_server_info;
> >
> > void drmSetServerInfo(drmServerInfoPtr info)
> > @@ -766,12 +768,7 @@ drmVersionPtr drmGetVersion(int fd)
> > drmVersionPtr retval;
> > drm_version_t *version = drmMalloc(sizeof(*version));
> >
> > - version->name_len = 0;
> > - version->name = NULL;
> > - version->date_len = 0;
> > - version->date = NULL;
> > - version->desc_len = 0;
> > - version->desc = NULL;
> > + memclear(version);
>
> I think this should be memclear(*version).
> Otherwise it clears the pointer not the structure.
Yeah ... and stupid me already pushed this :( I'll fix it up and have
another look at the patches to make sure I didn't botch another one.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] tests: remove intel-specific tests
2015-02-11 11:42 [PATCH 1/4] intel: Unconditionally clear ioctl structs Daniel Vetter
` (2 preceding siblings ...)
2015-02-11 11:42 ` [PATCH 4/4] xf86drm: Unconditionally clear ioctl structs Daniel Vetter
@ 2015-02-11 11:56 ` Daniel Vetter
2015-02-11 19:09 ` [PATCH 1/4] intel: Unconditionally clear ioctl structs Ian Romanick
4 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-11 11:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter, Daniel Vetter
These all moved to igt meanwhile.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
.gitignore | 4 --
tests/Makefile.am | 9 ----
tests/gem_basic.c | 102 ------------------------------------
tests/gem_flink.c | 137 -------------------------------------------------
tests/gem_mmap.c | 136 ------------------------------------------------
tests/gem_readwrite.c | 139 --------------------------------------------------
6 files changed, 527 deletions(-)
delete mode 100644 tests/gem_basic.c
delete mode 100644 tests/gem_flink.c
delete mode 100644 tests/gem_mmap.c
delete mode 100644 tests/gem_readwrite.c
diff --git a/.gitignore b/.gitignore
index 3596a4a8e4b9..06cc9283b494 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,10 +79,6 @@ tests/getclient
tests/getstats
tests/getversion
tests/lock
-tests/gem_basic
-tests/gem_flink
-tests/gem_mmap
-tests/gem_readwrite
tests/openclose
tests/setversion
tests/updatedraw
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e7ec4fa5ecff..37b8d3a1a271 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -58,15 +58,6 @@ TESTS = \
SUBDIRS += vbltest $(NULL)
-if HAVE_INTEL
-TESTS += \
- gem_basic \
- gem_flink \
- gem_readwrite \
- gem_mmap \
- $(NULL)
-endif
-
check_PROGRAMS += $(TESTS)
endif
diff --git a/tests/gem_basic.c b/tests/gem_basic.c
deleted file mode 100644
index 4e4b6cbdf1a9..000000000000
--- a/tests/gem_basic.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright © 2008 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- * Eric Anholt <eric@anholt.net>
- *
- */
-
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <assert.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <errno.h>
-#include <sys/stat.h>
-#include "drm.h"
-#include "i915_drm.h"
-
-static void
-test_bad_close(int fd)
-{
- struct drm_gem_close close;
- int ret;
-
- printf("Testing error return on bad close ioctl.\n");
-
- close.handle = 0x10101010;
- ret = ioctl(fd, DRM_IOCTL_GEM_CLOSE, &close);
-
- assert(ret == -1 && errno == EINVAL);
-}
-
-static void
-test_create_close(int fd)
-{
- struct drm_i915_gem_create create;
- struct drm_gem_close close;
- int ret;
-
- printf("Testing creating and closing an object.\n");
-
- memset(&create, 0, sizeof(create));
- create.size = 16 * 1024;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
-
- close.handle = create.handle;
- ret = ioctl(fd, DRM_IOCTL_GEM_CLOSE, &close);
-}
-
-static void
-test_create_fd_close(int fd)
-{
- struct drm_i915_gem_create create;
- int ret;
-
- printf("Testing closing with an object allocated.\n");
-
- memset(&create, 0, sizeof(create));
- create.size = 16 * 1024;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
-
- close(fd);
-}
-
-int main(int argc, char **argv)
-{
- int fd;
-
- fd = drm_open_matching("8086:*", 0);
- if (fd < 0) {
- fprintf(stderr, "failed to open intel drm device\n");
- return 0;
- }
-
- test_bad_close(fd);
- test_create_close(fd);
- test_create_fd_close(fd);
-
- return 0;
-}
diff --git a/tests/gem_flink.c b/tests/gem_flink.c
deleted file mode 100644
index ce43e42201ac..000000000000
--- a/tests/gem_flink.c
+++ /dev/null
@@ -1,137 +0,0 @@
-/*
- * Copyright © 2008 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- * Eric Anholt <eric@anholt.net>
- *
- */
-
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <assert.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <errno.h>
-#include <sys/stat.h>
-#include "drm.h"
-#include "i915_drm.h"
-
-static void
-test_flink(int fd)
-{
- struct drm_i915_gem_create create;
- struct drm_gem_flink flink;
- struct drm_gem_open open;
- int ret;
-
- printf("Testing flink and open.\n");
-
- memset(&create, 0, sizeof(create));
- create.size = 16 * 1024;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
-
- flink.handle = create.handle;
- ret = ioctl(fd, DRM_IOCTL_GEM_FLINK, &flink);
- assert(ret == 0);
-
- open.name = flink.name;
- ret = ioctl(fd, DRM_IOCTL_GEM_OPEN, &open);
- assert(ret == 0);
- assert(open.handle != 0);
-}
-
-static void
-test_double_flink(int fd)
-{
- struct drm_i915_gem_create create;
- struct drm_gem_flink flink;
- struct drm_gem_flink flink2;
- int ret;
-
- printf("Testing repeated flink.\n");
-
- memset(&create, 0, sizeof(create));
- create.size = 16 * 1024;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
-
- flink.handle = create.handle;
- ret = ioctl(fd, DRM_IOCTL_GEM_FLINK, &flink);
- assert(ret == 0);
-
- flink2.handle = create.handle;
- ret = ioctl(fd, DRM_IOCTL_GEM_FLINK, &flink2);
- assert(ret == 0);
- assert(flink2.name == flink.name);
-}
-
-static void
-test_bad_flink(int fd)
-{
- struct drm_gem_flink flink;
- int ret;
-
- printf("Testing error return on bad flink ioctl.\n");
-
- flink.handle = 0x10101010;
- ret = ioctl(fd, DRM_IOCTL_GEM_FLINK, &flink);
- assert(ret == -1 && errno == ENOENT);
-}
-
-static void
-test_bad_open(int fd)
-{
- struct drm_gem_open open;
- int ret;
-
- printf("Testing error return on bad open ioctl.\n");
-
- open.name = 0x10101010;
- ret = ioctl(fd, DRM_IOCTL_GEM_OPEN, &open);
-
- assert(ret == -1 && errno == ENOENT);
-}
-
-int main(int argc, char **argv)
-{
- int fd;
-
- if (geteuid()) {
- fprintf(stderr, "requires root privileges, skipping\n");
- return 77;
- }
-
- fd = drm_open_matching("8086:*", 0);
- if (fd < 0) {
- fprintf(stderr, "failed to open intel drm device, skipping\n");
- return 77;
- }
-
- test_flink(fd);
- test_double_flink(fd);
- test_bad_flink(fd);
- test_bad_open(fd);
-
- return 0;
-}
diff --git a/tests/gem_mmap.c b/tests/gem_mmap.c
deleted file mode 100644
index 2239789fade0..000000000000
--- a/tests/gem_mmap.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- * Copyright © 2008 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- * Eric Anholt <eric@anholt.net>
- *
- */
-
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <assert.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <errno.h>
-#include <sys/stat.h>
-#include "drm.h"
-#include "i915_drm.h"
-
-#define OBJECT_SIZE 16384
-
-int do_read(int fd, int handle, void *buf, int offset, int size)
-{
- struct drm_i915_gem_pread read;
-
- /* Ensure that we don't have any convenient data in buf in case
- * we fail.
- */
- memset(buf, 0xd0, size);
-
- memset(&read, 0, sizeof(read));
- read.handle = handle;
- read.data_ptr = (uintptr_t)buf;
- read.size = size;
- read.offset = offset;
-
- return ioctl(fd, DRM_IOCTL_I915_GEM_PREAD, &read);
-}
-
-int do_write(int fd, int handle, void *buf, int offset, int size)
-{
- struct drm_i915_gem_pwrite write;
-
- memset(&write, 0, sizeof(write));
- write.handle = handle;
- write.data_ptr = (uintptr_t)buf;
- write.size = size;
- write.offset = offset;
-
- return ioctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &write);
-}
-
-int main(int argc, char **argv)
-{
- int fd;
- struct drm_i915_gem_create create;
- struct drm_i915_gem_mmap mmap;
- struct drm_gem_close unref;
- uint8_t expected[OBJECT_SIZE];
- uint8_t buf[OBJECT_SIZE];
- uint8_t *addr;
- int ret;
- int handle;
-
- fd = drm_open_matching("8086:*", 0);
- if (fd < 0) {
- fprintf(stderr, "failed to open intel drm device, skipping\n");
- return 0;
- }
-
- memset(&mmap, 0, sizeof(mmap));
- mmap.handle = 0x10101010;
- mmap.offset = 0;
- mmap.size = 4096;
- printf("Testing mmaping of bad object.\n");
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap);
- assert(ret == -1 && errno == ENOENT);
-
- memset(&create, 0, sizeof(create));
- create.size = OBJECT_SIZE;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
- handle = create.handle;
-
- printf("Testing mmaping of newly created object.\n");
- mmap.handle = handle;
- mmap.offset = 0;
- mmap.size = OBJECT_SIZE;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap);
- assert(ret == 0);
- addr = (uint8_t *)(uintptr_t)mmap.addr_ptr;
-
- printf("Testing contents of newly created object.\n");
- memset(expected, 0, sizeof(expected));
- assert(memcmp(addr, expected, sizeof(expected)) == 0);
-
- printf("Testing coherency of writes and mmap reads.\n");
- memset(buf, 0, sizeof(buf));
- memset(buf + 1024, 0x01, 1024);
- memset(expected + 1024, 0x01, 1024);
- ret = do_write(fd, handle, buf, 0, OBJECT_SIZE);
- assert(ret == 0);
- assert(memcmp(buf, addr, sizeof(buf)) == 0);
-
- printf("Testing that mapping stays after close\n");
- unref.handle = handle;
- ret = ioctl(fd, DRM_IOCTL_GEM_CLOSE, &unref);
- assert(ret == 0);
- assert(memcmp(buf, addr, sizeof(buf)) == 0);
-
- printf("Testing unmapping\n");
- munmap(addr, OBJECT_SIZE);
-
- close(fd);
-
- return 0;
-}
diff --git a/tests/gem_readwrite.c b/tests/gem_readwrite.c
deleted file mode 100644
index 07dc853a0c24..000000000000
--- a/tests/gem_readwrite.c
+++ /dev/null
@@ -1,139 +0,0 @@
-/*
- * Copyright © 2008 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- * Eric Anholt <eric@anholt.net>
- *
- */
-
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <assert.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <errno.h>
-#include <sys/stat.h>
-#include "drm.h"
-#include "i915_drm.h"
-
-#define OBJECT_SIZE 16384
-
-int do_read(int fd, int handle, void *buf, int offset, int size)
-{
- struct drm_i915_gem_pread read;
-
- /* Ensure that we don't have any convenient data in buf in case
- * we fail.
- */
- memset(buf, 0xd0, size);
-
- memset(&read, 0, sizeof(read));
- read.handle = handle;
- read.data_ptr = (uintptr_t)buf;
- read.size = size;
- read.offset = offset;
-
- return ioctl(fd, DRM_IOCTL_I915_GEM_PREAD, &read);
-}
-
-int do_write(int fd, int handle, void *buf, int offset, int size)
-{
- struct drm_i915_gem_pwrite write;
-
- memset(&write, 0, sizeof(write));
- write.handle = handle;
- write.data_ptr = (uintptr_t)buf;
- write.size = size;
- write.offset = offset;
-
- return ioctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &write);
-}
-
-int main(int argc, char **argv)
-{
- int fd;
- struct drm_i915_gem_create create;
- uint8_t expected[OBJECT_SIZE];
- uint8_t buf[OBJECT_SIZE];
- int ret;
- int handle;
-
- fd = drm_open_matching("8086:*", 0);
- if (fd < 0) {
- fprintf(stderr, "failed to open intel drm device, skipping\n");
- return 0;
- }
-
- memset(&create, 0, sizeof(create));
- create.size = OBJECT_SIZE;
- ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
- assert(ret == 0);
- handle = create.handle;
-
- printf("Testing contents of newly created object.\n");
- ret = do_read(fd, handle, buf, 0, OBJECT_SIZE);
- assert(ret == 0);
- memset(&expected, 0, sizeof(expected));
- assert(memcmp(expected, buf, sizeof(expected)) == 0);
-
- printf("Testing read beyond end of buffer.\n");
- ret = do_read(fd, handle, buf, OBJECT_SIZE / 2, OBJECT_SIZE);
- printf("%d %d\n", ret, errno);
- assert(ret == -1 && errno == EINVAL);
-
- printf("Testing full write of buffer\n");
- memset(buf, 0, sizeof(buf));
- memset(buf + 1024, 0x01, 1024);
- memset(expected + 1024, 0x01, 1024);
- ret = do_write(fd, handle, buf, 0, OBJECT_SIZE);
- assert(ret == 0);
- ret = do_read(fd, handle, buf, 0, OBJECT_SIZE);
- assert(ret == 0);
- assert(memcmp(buf, expected, sizeof(buf)) == 0);
-
- printf("Testing partial write of buffer\n");
- memset(buf + 4096, 0x02, 1024);
- memset(expected + 4096, 0x02, 1024);
- ret = do_write(fd, handle, buf + 4096, 4096, 1024);
- assert(ret == 0);
- ret = do_read(fd, handle, buf, 0, OBJECT_SIZE);
- assert(ret == 0);
- assert(memcmp(buf, expected, sizeof(buf)) == 0);
-
- printf("Testing partial read of buffer\n");
- ret = do_read(fd, handle, buf, 512, 1024);
- assert(ret == 0);
- assert(memcmp(buf, expected + 512, 1024) == 0);
-
- printf("Testing read of bad buffer handle\n");
- ret = do_read(fd, 1234, buf, 0, 1024);
- assert(ret == -1 && errno == ENOENT);
-
- printf("Testing write of bad buffer handle\n");
- ret = do_write(fd, 1234, buf, 0, 1024);
- assert(ret == -1 && errno == ENOENT);
-
- close(fd);
-
- return 0;
-}
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] intel: Unconditionally clear ioctl structs
2015-02-11 11:42 [PATCH 1/4] intel: Unconditionally clear ioctl structs Daniel Vetter
` (3 preceding siblings ...)
2015-02-11 11:56 ` [PATCH] tests: remove intel-specific tests Daniel Vetter
@ 2015-02-11 19:09 ` Ian Romanick
4 siblings, 0 replies; 13+ messages in thread
From: Ian Romanick @ 2015-02-11 19:09 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
This patch is
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
On 02/11/2015 03:42 AM, Daniel Vetter wrote:
> We really have to do this to avoid surprises when extending the ABI
> later on. Especially when growing the structures.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> intel/intel_bufmgr_gem.c | 68 ++++++++++++++++++++++++------------------------
> 1 file changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index cf85bb8ae0bf..78875fd329f2 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -74,7 +74,7 @@
> #define VG(x)
> #endif
>
> -#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
> +#define memclear(s) memset(&s, 0, sizeof(s))
>
> #define DBG(...) do { \
> if (bufmgr_gem->bufmgr.debug) \
> @@ -593,7 +593,7 @@ drm_intel_gem_bo_busy(drm_intel_bo *bo)
> if (bo_gem->reusable && bo_gem->idle)
> return false;
>
> - VG_CLEAR(busy);
> + memclear(busy);
> busy.handle = bo_gem->gem_handle;
>
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> @@ -612,7 +612,7 @@ drm_intel_gem_bo_madvise_internal(drm_intel_bufmgr_gem *bufmgr_gem,
> {
> struct drm_i915_gem_madvise madv;
>
> - VG_CLEAR(madv);
> + memclear(madv);
> madv.handle = bo_gem->gem_handle;
> madv.madv = state;
> madv.retained = 1;
> @@ -741,7 +741,7 @@ retry:
>
> bo_gem->bo.size = bo_size;
>
> - VG_CLEAR(create);
> + memclear(create);
> create.size = bo_size;
>
> ret = drmIoctl(bufmgr_gem->fd,
> @@ -888,7 +888,7 @@ drm_intel_gem_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
>
> bo_gem->bo.size = size;
>
> - VG_CLEAR(userptr);
> + memclear(userptr);
> userptr.user_ptr = (__u64)((unsigned long)addr);
> userptr.user_size = size;
> userptr.flags = flags;
> @@ -972,7 +972,7 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr,
> }
> }
>
> - VG_CLEAR(open_arg);
> + memclear(open_arg);
> open_arg.name = handle;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_GEM_OPEN,
> @@ -1017,7 +1017,7 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr,
> bo_gem->global_name = handle;
> bo_gem->reusable = false;
>
> - VG_CLEAR(get_tiling);
> + memclear(get_tiling);
> get_tiling.handle = bo_gem->gem_handle;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GEM_GET_TILING,
> @@ -1060,7 +1060,7 @@ drm_intel_gem_bo_free(drm_intel_bo *bo)
> }
>
> /* Close this object */
> - VG_CLEAR(close);
> + memclear(close);
> close.handle = bo_gem->gem_handle;
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_CLOSE, &close);
> if (ret != 0) {
> @@ -1292,9 +1292,8 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable)
> DBG("bo_map: %d (%s), map_count=%d\n",
> bo_gem->gem_handle, bo_gem->name, bo_gem->map_count);
>
> - VG_CLEAR(mmap_arg);
> + memclear(mmap_arg);
> mmap_arg.handle = bo_gem->gem_handle;
> - mmap_arg.offset = 0;
> mmap_arg.size = bo->size;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GEM_MMAP,
> @@ -1316,7 +1315,7 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable)
> bo_gem->mem_virtual);
> bo->virtual = bo_gem->mem_virtual;
>
> - VG_CLEAR(set_domain);
> + memclear(set_domain);
> set_domain.handle = bo_gem->gem_handle;
> set_domain.read_domains = I915_GEM_DOMAIN_CPU;
> if (write_enable)
> @@ -1362,7 +1361,7 @@ map_gtt(drm_intel_bo *bo)
> DBG("bo_map_gtt: mmap %d (%s), map_count=%d\n",
> bo_gem->gem_handle, bo_gem->name, bo_gem->map_count);
>
> - VG_CLEAR(mmap_arg);
> + memclear(mmap_arg);
> mmap_arg.handle = bo_gem->gem_handle;
>
> /* Get the fake offset back... */
> @@ -1430,7 +1429,7 @@ drm_intel_gem_bo_map_gtt(drm_intel_bo *bo)
> * tell it when we're about to use things if we had done
> * rendering and it still happens to be bound to the GTT.
> */
> - VG_CLEAR(set_domain);
> + memclear(set_domain);
> set_domain.handle = bo_gem->gem_handle;
> set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> set_domain.write_domain = I915_GEM_DOMAIN_GTT;
> @@ -1529,7 +1528,7 @@ static int drm_intel_gem_bo_unmap(drm_intel_bo *bo)
> * Unlike GTT set domains, this only does work if the
> * buffer should be scanout-related.
> */
> - VG_CLEAR(sw_finish);
> + memclear(sw_finish);
> sw_finish.handle = bo_gem->gem_handle;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GEM_SW_FINISH,
> @@ -1571,7 +1570,7 @@ drm_intel_gem_bo_subdata(drm_intel_bo *bo, unsigned long offset,
> if (bo_gem->is_userptr)
> return -EINVAL;
>
> - VG_CLEAR(pwrite);
> + memclear(pwrite);
> pwrite.handle = bo_gem->gem_handle;
> pwrite.offset = offset;
> pwrite.size = size;
> @@ -1596,7 +1595,7 @@ drm_intel_gem_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id)
> struct drm_i915_get_pipe_from_crtc_id get_pipe_from_crtc_id;
> int ret;
>
> - VG_CLEAR(get_pipe_from_crtc_id);
> + memclear(get_pipe_from_crtc_id);
> get_pipe_from_crtc_id.crtc_id = crtc_id;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID,
> @@ -1626,7 +1625,7 @@ drm_intel_gem_bo_get_subdata(drm_intel_bo *bo, unsigned long offset,
> if (bo_gem->is_userptr)
> return -EINVAL;
>
> - VG_CLEAR(pread);
> + memclear(pread);
> pread.handle = bo_gem->gem_handle;
> pread.offset = offset;
> pread.size = size;
> @@ -1694,9 +1693,9 @@ drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns)
> }
> }
>
> + memclear(wait);
> wait.bo_handle = bo_gem->gem_handle;
> wait.timeout_ns = timeout_ns;
> - wait.flags = 0;
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
> if (ret == -1)
> return -errno;
> @@ -1719,7 +1718,7 @@ drm_intel_gem_bo_start_gtt_access(drm_intel_bo *bo, int write_enable)
> struct drm_i915_gem_set_domain set_domain;
> int ret;
>
> - VG_CLEAR(set_domain);
> + memclear(set_domain);
> set_domain.handle = bo_gem->gem_handle;
> set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> set_domain.write_domain = write_enable ? I915_GEM_DOMAIN_GTT : 0;
> @@ -2339,7 +2338,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used,
> */
> drm_intel_add_validate_buffer(bo);
>
> - VG_CLEAR(execbuf);
> + memclear(execbuf);
> execbuf.buffers_ptr = (uintptr_t) bufmgr_gem->exec_objects;
> execbuf.buffer_count = bufmgr_gem->exec_count;
> execbuf.batch_start_offset = 0;
> @@ -2426,7 +2425,7 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx,
> */
> drm_intel_add_validate_buffer2(bo, 0);
>
> - VG_CLEAR(execbuf);
> + memclear(execbuf);
> execbuf.buffers_ptr = (uintptr_t)bufmgr_gem->exec2_objects;
> execbuf.buffer_count = bufmgr_gem->exec_count;
> execbuf.batch_start_offset = 0;
> @@ -2517,7 +2516,7 @@ drm_intel_gem_bo_pin(drm_intel_bo *bo, uint32_t alignment)
> struct drm_i915_gem_pin pin;
> int ret;
>
> - VG_CLEAR(pin);
> + memclear(pin);
> pin.handle = bo_gem->gem_handle;
> pin.alignment = alignment;
>
> @@ -2540,7 +2539,7 @@ drm_intel_gem_bo_unpin(drm_intel_bo *bo)
> struct drm_i915_gem_unpin unpin;
> int ret;
>
> - VG_CLEAR(unpin);
> + memclear(unpin);
> unpin.handle = bo_gem->gem_handle;
>
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_UNPIN, &unpin);
> @@ -2696,7 +2695,7 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s
> DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
> pthread_mutex_unlock(&bufmgr_gem->lock);
>
> - VG_CLEAR(get_tiling);
> + memclear(get_tiling);
> get_tiling.handle = bo_gem->gem_handle;
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GEM_GET_TILING,
> @@ -2743,7 +2742,7 @@ drm_intel_gem_bo_flink(drm_intel_bo *bo, uint32_t * name)
> if (!bo_gem->global_name) {
> struct drm_gem_flink flink;
>
> - VG_CLEAR(flink);
> + memclear(flink);
> flink.handle = bo_gem->gem_handle;
>
> pthread_mutex_lock(&bufmgr_gem->lock);
> @@ -3078,7 +3077,7 @@ static int
> get_pci_device_id(drm_intel_bufmgr_gem *bufmgr_gem)
> {
> char *devid_override;
> - int devid;
> + int devid = 0;
> int ret;
> drm_i915_getparam_t gp;
>
> @@ -3090,8 +3089,7 @@ get_pci_device_id(drm_intel_bufmgr_gem *bufmgr_gem)
> }
> }
>
> - VG_CLEAR(devid);
> - VG_CLEAR(gp);
> + memclear(gp);
> gp.param = I915_PARAM_CHIPSET_ID;
> gp.value = &devid;
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
> @@ -3204,7 +3202,7 @@ drm_intel_gem_context_create(drm_intel_bufmgr *bufmgr)
> if (!context)
> return NULL;
>
> - VG_CLEAR(create);
> + memclear(create);
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create);
> if (ret != 0) {
> DBG("DRM_IOCTL_I915_GEM_CONTEXT_CREATE failed: %s\n",
> @@ -3229,7 +3227,7 @@ drm_intel_gem_context_destroy(drm_intel_context *ctx)
> if (ctx == NULL)
> return;
>
> - VG_CLEAR(destroy);
> + memclear(destroy);
>
> bufmgr_gem = (drm_intel_bufmgr_gem *)ctx->bufmgr;
> destroy.ctx_id = ctx->ctx_id;
> @@ -3255,7 +3253,7 @@ drm_intel_get_reset_stats(drm_intel_context *ctx,
> if (ctx == NULL)
> return -EINVAL;
>
> - memset(&stats, 0, sizeof(stats));
> + memclear(stats);
>
> bufmgr_gem = (drm_intel_bufmgr_gem *)ctx->bufmgr;
> stats.ctx_id = ctx->ctx_id;
> @@ -3285,7 +3283,7 @@ drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> struct drm_i915_reg_read reg_read;
> int ret;
>
> - VG_CLEAR(reg_read);
> + memclear(reg_read);
> reg_read.offset = offset;
>
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_REG_READ, ®_read);
> @@ -3390,7 +3388,7 @@ has_userptr(drm_intel_bufmgr_gem *bufmgr_gem)
> return false;
> }
>
> - memset(&userptr, 0, sizeof(userptr));
> + memclear(userptr);
> userptr.user_ptr = (__u64)(unsigned long)ptr;
> userptr.user_size = pgsz;
>
> @@ -3405,6 +3403,7 @@ retry:
> return false;
> }
>
> + memclear(close_bo);
> close_bo.handle = userptr.handle;
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_CLOSE, &close_bo);
> free(ptr);
> @@ -3451,6 +3450,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
> goto exit;
> }
>
> + memclear(aperture);
> ret = drmIoctl(bufmgr_gem->fd,
> DRM_IOCTL_I915_GEM_GET_APERTURE,
> &aperture);
> @@ -3500,7 +3500,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
> bufmgr_gem->gtt_size -= 256*1024*1024;
> }
>
> - VG_CLEAR(gp);
> + memclear(gp);
> gp.value = &tmp;
>
> gp.param = I915_PARAM_HAS_EXECBUF2;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread