Change function r535_gsp_msgq_recv_one_elem() return an error code
instead of a pointer to the buffer that was passed as a parameter.

r535_gsp_msgq_recv_one_elem() takes a struct r535_gsp_msg_info as
one of its parameter.  This struct manages the receive buffer for
incoming messages from GSP, and includes a pointer to that buffer.
On success, this function returns that pointer.

This behavior results in callers treating the returned pointer as
a new value, even though it's already known to the caller.
For example:

        buf = kvmalloc(max_t(u32, rpc->length, expected), GFP_KERNEL);
        info.gsp_rpc_buf = buf;
        buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
        if (IS_ERR(buf)) {
                kvfree(info.gsp_rpc_buf);
                info.gsp_rpc_buf = NULL;

It's not obvious, but 'buf' does not actually change its value,
unless r535_gsp_msgq_recv_one_elem fails.  This is why the caller
cannot do kvfree(buf).

Instead, r535_gsp_msgq_recv_one_elem() now returns an error code,
and 'buf' never changes value.

Signed-off-by: Timur Tabi <[email protected]>
---
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
index 0dc4782df8c0..50bd8407180e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/rpc.c
@@ -229,7 +229,7 @@ struct r535_gsp_msg_info {
 static void
 r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
 
-static void *
+static int
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
                            struct r535_gsp_msg_info *info)
 {
@@ -243,7 +243,7 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 
        ret = r535_gsp_msgq_wait(gsp, expected, info->retries);
        if (ret < 0)
-               return ERR_PTR(ret);
+               return ret;
 
        mqe = r535_gsp_msgq_get_entry(gsp);
 
@@ -254,7 +254,7 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
                        nvkm_error(&gsp->subdev,
                                   "Not a continuation of a large RPC\n");
                        r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
-                       return ERR_PTR(-EIO);
+                       return -EIO;
                }
        }
 
@@ -280,7 +280,7 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 
        mb();
        (*gsp->msgq.rptr) = rptr;
-       return buf;
+       return 0;
 }
 
 static void *
@@ -292,6 +292,7 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, 
int *retries)
        struct r535_gsp_msg_info info = {0};
        u32 expected = gsp_rpc_len;
        void *buf;
+       int ret;
 
        mqe = r535_gsp_msgq_get_entry(gsp);
        rpc = (struct nvfw_gsp_rpc *)mqe->data;
@@ -307,11 +308,10 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, 
int *retries)
        info.retries = retries;
        info.gsp_rpc_len = rpc->length;
 
-       buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
-       if (IS_ERR(buf)) {
-               kvfree(info.gsp_rpc_buf);
-               info.gsp_rpc_buf = NULL;
-               return buf;
+       ret = r535_gsp_msgq_recv_one_elem(gsp, &info);
+       if (ret) {
+               kvfree(buf);
+               return ERR_PTR(ret);
        }
 
        if (expected <= max_rpc_size)
@@ -332,10 +332,10 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, 
int *retries)
                info.gsp_rpc_len = rpc->length;
                info.continuation = true;
 
-               rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
-               if (IS_ERR_OR_NULL(rpc)) {
+               ret = r535_gsp_msgq_recv_one_elem(gsp, &info);
+               if (ret) {
                        kvfree(buf);
-                       return rpc;
+                       return ERR_PTR(ret);
                }
 
                size = info.gsp_rpc_len - sizeof(*rpc);
-- 
2.51.0

Reply via email to