On 31 August 2015 at 19:26, Chris Wilson chris@chris-wilson.co.uk wrote:
On Mon, Aug 31, 2015 at 07:14:12PM +0100, Emil Velikov wrote:
Just like we do for the original exec()
v2: move bo_gem declaration to the top of the function.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
intel/intel_bufmgr_gem.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 7303903..5287419 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -2185,10 +2185,14 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx, unsigned int flags) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; struct drm_i915_gem_execbuffer2 execbuf; int ret = 0; int i;if (bo_gem->has_error)return -ENOMEM;switch (flags & 0x7) { default: return -EINVAL;@@ -2259,8 +2263,7 @@ skip_execution: drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
drm_intel_bo_gem *bo_gem =(drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];Reusing bo_gem here is a little worrying as it would be very easy for someone to add code to the end of the function thinking that bo_gem still was the batch.
Doesn't this concert apply to drm_intel_gem_bo_exec() as well ?
If we had
static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo) { return (drm_intel_bo_gem *)bo; }
then we can start doing one offs like
if (to_bo_gem(bo)->has_error) return -ENOMEM;
and of course for (i = 0; i < bufmgr_gem->exec_count; i++) { drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
How about we do this as a follow up patch (4.1/17) that covers both functions ?
Thanks again, Emil