| Message ID | 1384939368-17291-1-git-send-email-zhenyuw@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote: > If valgrind is not available, current VG_CLEAR() would just ignore > memory clear operation which might make invalid ioctl argument. So > make sure VG_CLEAR() will always clear memory. > --- > intel/intel_bufmgr_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index df6fcec..389f73a 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 VG_CLEAR(s) (memset(&s, 0, sizeof(s))) VG_CLEAR() is really just for valgrind. If you need to set some specific variable/field to 0 then you need to set it to 0 and not rely on VG_CLEAR() to do it for you. What's the actual issue you have?
On 2013.11.20 11:36:20 +0000, Damien Lespiau wrote: > On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote: > > If valgrind is not available, current VG_CLEAR() would just ignore > > memory clear operation which might make invalid ioctl argument. So > > make sure VG_CLEAR() will always clear memory. > > --- > > intel/intel_bufmgr_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > index df6fcec..389f73a 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 VG_CLEAR(s) (memset(&s, 0, sizeof(s))) > > VG_CLEAR() is really just for valgrind. If you need to set some specific > variable/field to 0 then you need to set it to 0 and not rely on > VG_CLEAR() to do it for you. > ok, in valgrind case it does memory clear for ioctl args which I think is good behavior in non-valgrind case too. > What's the actual issue you have? > During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats argument is not cleared, which failed in kernel check.
On Wed, Nov 20, 2013 at 11:53:54PM +0800, Zhenyu Wang wrote: > > VG_CLEAR() is really just for valgrind. If you need to set some specific > > variable/field to 0 then you need to set it to 0 and not rely on > > VG_CLEAR() to do it for you. > > > > ok, in valgrind case it does memory clear for ioctl args which I think is > good behavior in non-valgrind case too. Ian just submitted a patch that memset the whole structure. > > What's the actual issue you have? > > > > During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats > argument is not cleared, which failed in kernel check. Right, so the fix is (was) to zero the fields checked by the kernel explicitely, not change the VG() macro, which is just used in testing (and it should this way). HTH,
On 2013.11.20 16:59:22 +0000, Damien Lespiau wrote: > Right, so the fix is (was) to zero the fields checked by the kernel > explicitely, not change the VG() macro, which is just used in testing > (and it should this way). > That's fine. Thanks.
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index df6fcec..389f73a 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 VG_CLEAR(s) (memset(&s, 0, sizeof(s))) #define DBG(...) do { \ if (bufmgr_gem->bufmgr.debug) \