diff mbox

[11/11] Propagate correctly errors from qxlhw_handle_to_bo

Message ID 1432721046-4418-12-git-send-email-fziglio@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Frediano Ziglio May 27, 2015, 10:04 a.m. UTC
This function could return a NULL pointer in case of handle not
present and in case of out of memory conditions however caller
function always returned EINVAL error hiding a possible ENOMEM.
This patch change the function to return the error instead to
be able to propagate the error instead of assuming EINVAL.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 qxl/qxl_ioctl.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

Comments

Dave Airlie May 28, 2015, 3:33 a.m. UTC | #1
On 27 May 2015 at 20:04, Frediano Ziglio <fziglio@redhat.com> wrote:
> This function could return a NULL pointer in case of handle not
> present and in case of out of memory conditions however caller
> function always returned EINVAL error hiding a possible ENOMEM.
> This patch change the function to return the error instead to
> be able to propagate the error instead of assuming EINVAL.
>
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>

Reviewed-by: Dave Airlie <airlied@redhat.com>
> ---
>  qxl/qxl_ioctl.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/qxl/qxl_ioctl.c b/qxl/qxl_ioctl.c
> index bb326ff..37f1faf 100644
> --- a/qxl/qxl_ioctl.c
> +++ b/qxl/qxl_ioctl.c
> @@ -107,9 +107,9 @@ apply_surf_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
>  }
>
>  /* return holding the reference to this object */
> -static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device *qdev,
> -                                        struct drm_file *file_priv, uint64_t handle,
> -                                        struct qxl_release *release)
> +static int qxlhw_handle_to_bo(struct qxl_device *qdev,
> +                             struct drm_file *file_priv, uint64_t handle,
> +                             struct qxl_release *release, struct qxl_bo **qbo_p)
>  {
>         struct drm_gem_object *gobj;
>         struct qxl_bo *qobj;
> @@ -117,16 +117,17 @@ static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device *qdev,
>
>         gobj = drm_gem_object_lookup(qdev->ddev, file_priv, handle);
>         if (!gobj)
> -               return NULL;
> +               return -EINVAL;
>
>         qobj = gem_to_qxl_bo(gobj);
>
>         ret = qxl_release_list_add(release, qobj);
>         drm_gem_object_unreference_unlocked(gobj);
>         if (ret)
> -               return NULL;
> +               return ret;
>
> -       return qobj;
> +       *qbo_p = qobj;
> +       return 0;
>  }
>
>  /*
> @@ -219,13 +220,10 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>                 reloc_info[i].type = reloc.reloc_type;
>
>                 if (reloc.dst_handle) {
> -                       reloc_info[i].dst_bo = qxlhw_handle_to_bo(qdev, file_priv,
> -                                                                 reloc.dst_handle, release);
> -                       if (!reloc_info[i].dst_bo) {
> -                               ret = -EINVAL;
> -                               reloc_info[i].src_bo = NULL;
> +                       ret = qxlhw_handle_to_bo(qdev, file_priv, reloc.dst_handle, release,
> +                                                &reloc_info[i].dst_bo);
> +                       if (ret)
>                                 goto out_free_bos;
> -                       }
>                         reloc_info[i].dst_offset = reloc.dst_offset;
>                 } else {
>                         reloc_info[i].dst_bo = cmd_bo;
> @@ -234,14 +232,11 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>                 num_relocs++;
>
>                 /* reserve and validate the reloc dst bo */
> -               if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle > 0) {
> -                       reloc_info[i].src_bo =
> -                               qxlhw_handle_to_bo(qdev, file_priv,
> -                                                  reloc.src_handle, release);
> -                       if (!reloc_info[i].src_bo) {
> -                               ret = -EINVAL;
> +               if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
> +                       ret = qxlhw_handle_to_bo(qdev, file_priv, reloc.src_handle, release,
> +                                                &reloc_info[i].src_bo);
> +                       if (ret)
>                                 goto out_free_bos;
> -                       }
>                         reloc_info[i].src_offset = reloc.src_offset;
>                 } else {
>                         reloc_info[i].src_bo = NULL;
> --
> 2.1.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
diff mbox

Patch

diff --git a/qxl/qxl_ioctl.c b/qxl/qxl_ioctl.c
index bb326ff..37f1faf 100644
--- a/qxl/qxl_ioctl.c
+++ b/qxl/qxl_ioctl.c
@@ -107,9 +107,9 @@  apply_surf_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
 }
 
 /* return holding the reference to this object */
-static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device *qdev,
-					 struct drm_file *file_priv, uint64_t handle,
-					 struct qxl_release *release)
+static int qxlhw_handle_to_bo(struct qxl_device *qdev,
+			      struct drm_file *file_priv, uint64_t handle,
+			      struct qxl_release *release, struct qxl_bo **qbo_p)
 {
 	struct drm_gem_object *gobj;
 	struct qxl_bo *qobj;
@@ -117,16 +117,17 @@  static struct qxl_bo *qxlhw_handle_to_bo(struct qxl_device *qdev,
 
 	gobj = drm_gem_object_lookup(qdev->ddev, file_priv, handle);
 	if (!gobj)
-		return NULL;
+		return -EINVAL;
 
 	qobj = gem_to_qxl_bo(gobj);
 
 	ret = qxl_release_list_add(release, qobj);
 	drm_gem_object_unreference_unlocked(gobj);
 	if (ret)
-		return NULL;
+		return ret;
 
-	return qobj;
+	*qbo_p = qobj;
+	return 0;
 }
 
 /*
@@ -219,13 +220,10 @@  static int qxl_process_single_command(struct qxl_device *qdev,
 		reloc_info[i].type = reloc.reloc_type;
 
 		if (reloc.dst_handle) {
-			reloc_info[i].dst_bo = qxlhw_handle_to_bo(qdev, file_priv,
-								  reloc.dst_handle, release);
-			if (!reloc_info[i].dst_bo) {
-				ret = -EINVAL;
-				reloc_info[i].src_bo = NULL;
+			ret = qxlhw_handle_to_bo(qdev, file_priv, reloc.dst_handle, release,
+						 &reloc_info[i].dst_bo);
+			if (ret)
 				goto out_free_bos;
-			}
 			reloc_info[i].dst_offset = reloc.dst_offset;
 		} else {
 			reloc_info[i].dst_bo = cmd_bo;
@@ -234,14 +232,11 @@  static int qxl_process_single_command(struct qxl_device *qdev,
 		num_relocs++;
 
 		/* reserve and validate the reloc dst bo */
-		if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle > 0) {
-			reloc_info[i].src_bo =
-				qxlhw_handle_to_bo(qdev, file_priv,
-						   reloc.src_handle, release);
-			if (!reloc_info[i].src_bo) {
-				ret = -EINVAL;
+		if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
+			ret = qxlhw_handle_to_bo(qdev, file_priv, reloc.src_handle, release,
+						 &reloc_info[i].src_bo);
+			if (ret)
 				goto out_free_bos;
-			}
 			reloc_info[i].src_offset = reloc.src_offset;
 		} else {
 			reloc_info[i].src_bo = NULL;