Skip to content

libnvme: return libnvmf_tid_from_fields() as int, not a pointer - #3669

Open
martin-belanger wants to merge 1 commit into
linux-nvme:masterfrom
martin-belanger:tid-ctors-return-int
Open

libnvme: return libnvmf_tid_from_fields() as int, not a pointer#3669
martin-belanger wants to merge 1 commit into
linux-nvme:masterfrom
martin-belanger:tid-ctors-return-int

Conversation

@martin-belanger

Copy link
Copy Markdown

libnvmf_tid_from_fields() returned NULL on any failure, so a caller could not tell a validation error (-EINVAL, non-numeric traddr/host_traddr) from an allocation failure (-ENOMEM). Two internal callers actually got this wrong: fabrics.c's build_conn_tid() reported every failure as -EINVAL, and libnvme/src/nvme/fabrics.c's nvmf_sanitize_addrs() and the owner lookup in libnvmf_get_owner_from_fctx() reported every failure as -ENOMEM.

Convert to int with an out-param, matching libnvmf_config_read()'s existing convention, and update every call site. The SWIG binding now raises a real errno-based message instead of always PyErr_NoMemory().

Also check for a partial shr_xstrdup() failure across the struct's multi-field copy, which the pointer-returning version never checked.

libnvmf_tid_from_fields() returned NULL on any failure, so a caller could
not tell a validation error (-EINVAL, non-numeric traddr/host_traddr) from
an allocation failure (-ENOMEM). Two internal callers actually got this
wrong: fabrics.c's build_conn_tid() reported every failure as -EINVAL, and
libnvme/src/nvme/fabrics.c's nvmf_sanitize_addrs() and the owner lookup in
libnvmf_get_owner_from_fctx() reported every failure as -ENOMEM.

Convert to int with an out-param, matching libnvmf_config_read()'s
existing convention, and update every call site. The SWIG binding now
raises a real errno-based message instead of always PyErr_NoMemory().

Also check for a partial shr_xstrdup() failure across the struct's
multi-field copy, which the pointer-returning version never checked.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
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.

1 participant