On 1/10/24 20:50, Markus Elfring wrote:
From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> Date: Wed, 10 Jan 2024 21:15:48 +0100 The label “err” was used to jump to a kfree() call despite of the detail in the implementation of the function “io_ring_ctx_alloc” that it was determined already that a corresponding variable contained a null pointer because of a failed memory allocation.
It's _much_ simpler the way it currently is, compare it with maintaining a bunch of labels. That is the advantage of being able to distinguish un-allocated state like NULL, just kfree them and don't care about jumping to a wrong one or keeping them in order.
1. Thus use more appropriate labels instead. 2. Reorder jump targets at the end. Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> --- See also: https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources io_uring/io_uring.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index c9a63c39cdd0..7727cdd505ae 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -295,12 +295,14 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) hash_bits = ilog2(p->cq_entries) - 5; hash_bits = clamp(hash_bits, 1, 8); if (io_alloc_hash_table(&ctx->cancel_table, hash_bits)) - goto err; + goto destroy_io_bl_xa; + if (io_alloc_hash_table(&ctx->cancel_table_locked, hash_bits)) - goto err; + goto free_cancel_table_hbs; + if (percpu_ref_init(&ctx->refs, io_ring_ctx_ref_free, 0, GFP_KERNEL)) - goto err; + goto free_cancel_table_locked_hbs; ctx->flags = p->flags; init_waitqueue_head(&ctx->sqo_sq_wait); @@ -341,9 +343,12 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) INIT_WQ_LIST(&ctx->submit_state.compl_reqs); INIT_HLIST_HEAD(&ctx->cancelable_uring_cmd); return ctx; -err: - kfree(ctx->cancel_table.hbs); + +free_cancel_table_locked_hbs: kfree(ctx->cancel_table_locked.hbs); +free_cancel_table_hbs: + kfree(ctx->cancel_table.hbs); +destroy_io_bl_xa: xa_destroy(&ctx->io_bl_xa); kfree(ctx); return NULL; -- 2.43.0
-- Pavel Begunkov
