* [PATCH] drm: Use ENOENT consistently for the error return for an unmatched handle.
@ 2010-08-04 13:19 Chris Wilson
2010-08-04 22:46 ` Dave Airlie
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2010-08-04 13:19 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
This is consistent with trying to access a filename that not exist
within a directory which is a good analogy here. The main reason for the
change is that it is easy to confuse the error code of EBADF as an
performing an ioctl on an invalid file descriptor (rather than an
unknown object).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 26 +++++++++++++-------------
drivers/gpu/drm/i915/i915_gem_tiling.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_gem.c | 8 ++++----
drivers/gpu/drm/nouveau/nv04_crtc.c | 2 +-
drivers/gpu/drm/nouveau/nv50_crtc.c | 2 +-
drivers/gpu/drm/radeon/radeon_cs.c | 2 +-
drivers/gpu/drm/radeon/radeon_cursor.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 12 ++++++------
9 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4f1b867..bf92d07 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -322,7 +322,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
again:
if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0f0c387..862ce34 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -456,7 +456,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
obj_priv = to_intel_bo(obj);
/* Bounds check source.
@@ -919,7 +919,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
obj_priv = to_intel_bo(obj);
/* Bounds check destination.
@@ -1002,7 +1002,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
obj_priv = to_intel_bo(obj);
mutex_lock(&dev->struct_mutex);
@@ -1064,7 +1064,7 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL) {
mutex_unlock(&dev->struct_mutex);
- return -EBADF;
+ return -ENOENT;
}
#if WATCH_BUF
@@ -1103,7 +1103,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
offset = args->offset;
@@ -1380,7 +1380,7 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EBADF;
+ return -ENOENT;
mutex_lock(&dev->struct_mutex);
@@ -3165,7 +3165,7 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
reloc->target_handle);
if (target_obj == NULL) {
i915_gem_object_unpin(obj);
- return -EBADF;
+ return -ENOENT;
}
target_obj_priv = to_intel_bo(target_obj);
@@ -3580,7 +3580,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
exec_list[i].handle, i);
/* prevent error path from reading uninitialized data */
args->buffer_count = i + 1;
- ret = -EBADF;
+ ret = -ENOENT;
goto err;
}
@@ -3590,7 +3590,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
object_list[i]);
/* prevent error path from reading uninitialized data */
args->buffer_count = i + 1;
- ret = -EBADF;
+ ret = -EINVAL;
goto err;
}
obj_priv->in_execbuffer = true;
@@ -4067,7 +4067,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
DRM_ERROR("Bad handle in i915_gem_pin_ioctl(): %d\n",
args->handle);
mutex_unlock(&dev->struct_mutex);
- return -EBADF;
+ return -ENOENT;
}
obj_priv = to_intel_bo(obj);
@@ -4123,7 +4123,7 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data,
DRM_ERROR("Bad handle in i915_gem_unpin_ioctl(): %d\n",
args->handle);
mutex_unlock(&dev->struct_mutex);
- return -EBADF;
+ return -ENOENT;
}
obj_priv = to_intel_bo(obj);
@@ -4157,7 +4157,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_busy_ioctl(): %d\n",
args->handle);
- return -EBADF;
+ return -ENOENT;
}
mutex_lock(&dev->struct_mutex);
@@ -4210,7 +4210,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
if (obj == NULL) {
DRM_ERROR("Bad handle in i915_gem_madvise_ioctl(): %d\n",
args->handle);
- return -EBADF;
+ return -ENOENT;
}
mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 155719e..710eca7 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -275,7 +275,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EINVAL;
+ return -ENOENT;
obj_priv = to_intel_bo(obj);
if (!i915_tiling_ok(dev, args->stride, obj->size, args->tiling_mode)) {
@@ -362,7 +362,7 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
obj = drm_gem_object_lookup(dev, file_priv, args->handle);
if (obj == NULL)
- return -EINVAL;
+ return -ENOENT;
obj_priv = to_intel_bo(obj);
mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 547f2c2..0f417ac 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -284,7 +284,7 @@ retry:
if (!gem) {
NV_ERROR(dev, "Unknown handle 0x%08x\n", b->handle);
validate_fini(op, NULL);
- return -EINVAL;
+ return -ENOENT;
}
nvbo = gem->driver_private;
@@ -759,7 +759,7 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void *data,
gem = drm_gem_object_lookup(dev, file_priv, req->handle);
if (!gem)
- return ret;
+ return -ENOENT;
nvbo = nouveau_gem_object(gem);
if (nvbo->cpu_filp) {
@@ -797,7 +797,7 @@ nouveau_gem_ioctl_cpu_fini(struct drm_device *dev, void *data,
gem = drm_gem_object_lookup(dev, file_priv, req->handle);
if (!gem)
- return ret;
+ return -ENOENT;
nvbo = nouveau_gem_object(gem);
if (nvbo->cpu_filp != file_priv)
@@ -822,7 +822,7 @@ nouveau_gem_ioctl_info(struct drm_device *dev, void *data,
gem = drm_gem_object_lookup(dev, file_priv, req->handle);
if (!gem)
- return -EINVAL;
+ return -ENOENT;
ret = nouveau_gem_info(gem, req);
drm_gem_object_unreference_unlocked(gem);
diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index 1c20c08..840da8e 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -914,7 +914,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
gem = drm_gem_object_lookup(dev, file_priv, buffer_handle);
if (!gem)
- return -EINVAL;
+ return -ENOENT;
cursor = nouveau_gem_object(gem);
ret = nouveau_bo_map(cursor);
diff --git a/drivers/gpu/drm/nouveau/nv50_crtc.c b/drivers/gpu/drm/nouveau/nv50_crtc.c
index 5d11ea1..c7c94a0 100644
--- a/drivers/gpu/drm/nouveau/nv50_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv50_crtc.c
@@ -348,7 +348,7 @@ nv50_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
gem = drm_gem_object_lookup(dev, file_priv, buffer_handle);
if (!gem)
- return -EINVAL;
+ return -ENOENT;
cursor = nouveau_gem_object(gem);
ret = nouveau_bo_map(cursor);
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index ae0fb73..fcc79b5 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -72,7 +72,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
if (p->relocs[i].gobj == NULL) {
DRM_ERROR("gem object lookup failed 0x%x\n",
r->handle);
- return -EINVAL;
+ return -ENOENT;
}
p->relocs_ptr[i] = &p->relocs[i];
p->relocs[i].robj = p->relocs[i].gobj->driver_private;
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c
index 4eb67c0..5731fc9 100644
--- a/drivers/gpu/drm/radeon/radeon_cursor.c
+++ b/drivers/gpu/drm/radeon/radeon_cursor.c
@@ -170,7 +170,7 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc,
obj = drm_gem_object_lookup(crtc->dev, file_priv, handle);
if (!obj) {
DRM_ERROR("Cannot find cursor object %x for crtc %d\n", handle, radeon_crtc->crtc_id);
- return -EINVAL;
+ return -ENOENT;
}
ret = radeon_gem_object_pin(obj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr);
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index a72a3ee..c578f26 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -226,7 +226,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
/* just do a BO wait for now */
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL) {
- return -EINVAL;
+ return -ENOENT;
}
robj = gobj->driver_private;
@@ -245,7 +245,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data,
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL) {
- return -EINVAL;
+ return -ENOENT;
}
robj = gobj->driver_private;
args->addr_ptr = radeon_bo_mmap_offset(robj);
@@ -264,7 +264,7 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL) {
- return -EINVAL;
+ return -ENOENT;
}
robj = gobj->driver_private;
r = radeon_bo_wait(robj, &cur_placement, true);
@@ -294,7 +294,7 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL) {
- return -EINVAL;
+ return -ENOENT;
}
robj = gobj->driver_private;
r = radeon_bo_wait(robj, NULL, false);
@@ -316,7 +316,7 @@ int radeon_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
DRM_DEBUG("%d \n", args->handle);
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL)
- return -EINVAL;
+ return -ENOENT;
robj = gobj->driver_private;
r = radeon_bo_set_tiling_flags(robj, args->tiling_flags, args->pitch);
drm_gem_object_unreference_unlocked(gobj);
@@ -334,7 +334,7 @@ int radeon_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
DRM_DEBUG("\n");
gobj = drm_gem_object_lookup(dev, filp, args->handle);
if (gobj == NULL)
- return -EINVAL;
+ return -ENOENT;
rbo = gobj->driver_private;
r = radeon_bo_reserve(rbo, false);
if (unlikely(r != 0))
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: Use ENOENT consistently for the error return for an unmatched handle.
2010-08-04 13:19 [PATCH] drm: Use ENOENT consistently for the error return for an unmatched handle Chris Wilson
@ 2010-08-04 22:46 ` Dave Airlie
2010-08-05 8:25 ` Chris Wilson
0 siblings, 1 reply; 3+ messages in thread
From: Dave Airlie @ 2010-08-04 22:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, dri-devel
On Wed, 2010-08-04 at 14:19 +0100, Chris Wilson wrote:
> This is consistent with trying to access a filename that not exist
> within a directory which is a good analogy here. The main reason for the
> change is that it is easy to confuse the error code of EBADF as an
> performing an ioctl on an invalid file descriptor (rather than an
> unknown object).
Have you verified no userspace relies on this return value? since this
technically an ABI change.
>From what I can see probably only libdrm tests care.
Dave.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: Use ENOENT consistently for the error return for an unmatched handle.
2010-08-04 22:46 ` Dave Airlie
@ 2010-08-05 8:25 ` Chris Wilson
0 siblings, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2010-08-05 8:25 UTC (permalink / raw)
To: Dave Airlie; +Cc: intel-gfx, dri-devel
On Thu, 05 Aug 2010 08:46:31 +1000, Dave Airlie <airlied@redhat.com> wrote:
> Have you verified no userspace relies on this return value? since this
> technically an ABI change.
>
> >From what I can see probably only libdrm tests care.
I haven't found any other instances of code checking return values, more
often the error code is simply reported. Even those tests show that we can
expect EINVAL, ENOENT or EBADF for an invalid buffer handle.
It's the reporting that I want clarified as the "invalid fd" is misleading
for bug reporters. (Doubly so when this gets confused with a genuine
EBADF!)
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-08-05 8:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-04 13:19 [PATCH] drm: Use ENOENT consistently for the error return for an unmatched handle Chris Wilson
2010-08-04 22:46 ` Dave Airlie
2010-08-05 8:25 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.