Qiskit / qiskit / 36406736678

88%

Build Type

push

github

Committed by web-flow

Commit Message

Make null-freeing uniform across the C API `qk_*_free` functions (#17067)

* Make null-freeing uniform across the C API `qk_*_free` functions

The C API's `qk_*_free` functions all accept a null pointer and do nothing
with it, but their safety documentation says that in three different ways.
One group states null is fine, a second requires a "valid, non-null
pointer" -- which reads as undefined behavior on null -- and `qk_str_free`
and `qk_control_flow_instruction_free` never mention null at all.  The
implementations are consistent; only the documentation is not.  Twelve of
the thirteen free functions already refuse null, using the guard that
`crates/cext/src/pointers.rs` documents as the canonical template for the
crate.

That inconsistency matters most to bindings that free in a destructor or
finalizer, which routinely reach a free call with a pointer that may be
null: an object whose construction failed, or an optional out-parameter
that was never written.  Whether that is legal currently depends on which
of the three wordings a given function happens to use, so the safe move is
to add a null check at every call site even where the implementation
already has one.  Qiskit.jl calls `qk_str_free` with a null pointer after
every successful transpile, because the error-string out-parameter is only
written on failure.  That has never misbehaved, but the documentation says
it is undefined.

Adopt the "either null or a valid pointer" wording for every free
function: update the four docstrings that currently claim null is
undefined behavior, and add an explicit null sentence to the two that are
silent on it.  `qk_control_flow_instruction_free` is also the one free
function that does not guard -- it passes the pointer straight to
`Box::from_raw` -- so null is genuinely undefined there; give it the same
guard as the rest of the crate.  That makes the guarantee uniform rather
than something callers check function by function.  A unit test calls it
with a null ... (continued)

Coverage Stats

8 of 8 new or added lines in 1 file covered. (100.0%)

12 existing lines in 2 files now uncovered.

122374 of 138882 relevant lines covered (88.11%)

944055.3 hits per line