On 3/21/23 09:33, Cédric Le Goater wrote:
+static void aio_bh_slice_insert(AioContext *ctx, BHListSlice *slice) +{ + QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, slice, next); +} + /* Multiple occurrences of aio_bh_poll cannot be called concurrently. */ int aio_bh_poll(AioContext *ctx) { @@ -164,7 +169,13 @@ int aio_bh_poll(AioContext *ctx)/* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list); - QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next); + + /* + * GCC13 [-Werror=dangling-pointer=] complains that the local variable + * 'slice' is being stored in a global list in 'ctx->bh_slice_list'. + * Use a helper to silent the compiler + */ + aio_bh_slice_insert(ctx, &slice);while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {QEMUBH *bh; --
Sorry, but an API that has "insert" and not "remove", and where the argument is *expected to be* a local variable (which must be removed to avoid a dangling pointer---and the warning is exactly -Wdangling-pointer), ranks at least -7 in the bad API ranking[1].
I tried wrapping the BHListSlice and BHListSlice* into an iterator struct (which is also really overkill, but at least---in theory---it's idiomatic), but the code was hard to follow.
The fact that the workaround is so ugly, in my opinion, points even more strongly at the compiler being in the wrong here.
Paolo [1] http://sweng.the-davies.net/Home/rustys-api-design-manifesto
