Skip to content

dht: unwind mknod and symlink with their own FOP token - #4757

Open
ThalesBarretto wants to merge 1 commit into
gluster:develfrom
ThalesBarretto:dht-h18-fop-token
Open

ThalesBarretto wants to merge 1 commit into
gluster:develfrom
ThalesBarretto:dht-h18-fop-token

Conversation

@ThalesBarretto

@ThalesBarretto ThalesBarretto commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Fixes: #4756

What

dht_newfile_cbk is the callback of both dht_mknod and dht_symlink, but it always unwound the frame with
DHT_STACK_UNWIND(mknod, ...), and the error path of dht_symlink unwound with the token of the neighbouring
dht_link. The FOP token only selects which fop_*_cbk_t typedef STACK_UNWIND_STRICT casts frame->ret to;
fop_mknod_cbk_t, fop_symlink_cbk_t and fop_link_cbk_t have identical signatures, so there is no behaviour
change
— what the wrong token defeats is the compile-time signature check STACK_UNWIND_STRICT exists for.

This dispatches on local->fop in dht_newfile_cbk, the way dht_file_setxattr_cbk, dht_file_removexattr_cbk
and dht_common_xattrop_cbk already do for their path/fd pairs, and gives dht_symlink's error path its own
symlink token. Both wrong tokens came from a310d0e6b0 ("cluster/dht: Change STACK_UNWIND to
STACK_UNWIND_STRICT", 2009), a bulk conversion that copied the token of the adjacent function.

Scope / relation to #4755

Behaviour-neutral type-safety cleanup, split out of #4755 (the decommissioned-brick lock-leak fix) so that PR
carries only the functional change. It touches a different region of dht_newfile_cbk than #4755 and the two merge
in any order. One detail for a careful reader: in dht_newfile_cbk the op_ret == -1 path reaches out: with
local still NULL (it is read after the early exit), so the new symlink dispatch is effective on the success path,
and the error-path unwind keeps falling through to the identical mknod cast until #4755's local hoist lands —
no observable difference either way, the typedefs being identical.

Verification

  • fop_mknod_cbk_t, fop_symlink_cbk_t and fop_link_cbk_t are byte-identical typedefs, and
    STACK_UNWIND_STRICT invokes frame->ret regardless of the token, so nothing changes at runtime — this is a
    type-clarity / compile-time-check fix.
  • Builds without warnings.

dht_newfile_cbk is the callback of both dht_mknod and dht_symlink but
always unwound with DHT_STACK_UNWIND(mknod, ...), and the error path of
dht_symlink unwound with the token of the neighbouring dht_link. The
token only selects the fop_*_cbk_t typedef STACK_UNWIND_STRICT casts
frame->ret to; fop_mknod_cbk_t, fop_symlink_cbk_t and fop_link_cbk_t
have the same signature, so there is no behaviour change. What the wrong
token defeats is the compile-time check STACK_UNWIND_STRICT exists for.

Both came from a310d0e ("cluster/dht: Change STACK_UNWIND to
STACK_UNWIND_STRICT", 2009), a bulk conversion that copied the token of
the adjacent function. Dispatch on local->fop, the way
dht_file_setxattr_cbk, dht_file_removexattr_cbk and
dht_common_xattrop_cbk already do for their path/fd pairs.

Fixes: gluster#4756
Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
@ThalesBarretto
ThalesBarretto marked this pull request as ready for review September 11, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dht: dht_newfile_cbk and dht_symlink unwind through the wrong fop_*_cbk_t typedef

2 participants