On 2021-12-17 4:00 PM, Dan Carpenter wrote:
This code frees "graph" and then dereferences to save the error code. Save the error code first and then use gotos to unwind the allocation.
Fixes: 59716aa3f976 ("ASoC: qdsp6: Fix an IS_ERR() vs NULL bug") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
sound/soc/qcom/qdsp6/q6apm.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c index 3e007d609a9b..f424d7aa389a 100644 --- a/sound/soc/qcom/qdsp6/q6apm.c +++ b/sound/soc/qcom/qdsp6/q6apm.c @@ -615,7 +615,7 @@ struct q6apm_graph *q6apm_graph_open(struct device *dev, q6apm_cb cb, graph = kzalloc(sizeof(*graph), GFP_KERNEL); if (!graph) { ret = -ENOMEM;
goto err;
goto put_ar_graph;}
graph->apm = apm;
@@ -631,13 +631,15 @@ struct q6apm_graph *q6apm_graph_open(struct device *dev, q6apm_cb cb,
graph->port = gpr_alloc_port(apm->gdev, dev, graph_callback, graph); if (IS_ERR(graph->port)) {
kfree(graph);goto err;
goto free_graph;}
return graph;
-err:
+free_graph:
- kfree(graph);
+put_ar_graph:
Hello Dan,
The patch looks good! My only suggestion is a readability improvement, but I'm unaware of the convention chosen for qcom directory so you may choose to ignore it:
Function q6apm_graph_open() has two separate return paths: a happy path ending in 'return graph' and an error path which eventually ends with 'return ERR_PTR(ret)'. Current goto label-naming convention suggests it's a happy path nonetheless.
s/free_graph/err_alloc_port/ and s/put_ar_graph/err_alloc_graph/ tells reader upfront that they are in the error path.
Regards, Czarek
kref_put(&ar_graph->refcount, q6apm_put_audioreach_graph); return ERR_PTR(ret); }