On 3/5/26 8:59 AM, Krzysztof Kozlowski wrote:
On 02/03/2026 20:12, Andrew Davis wrote:
On 2/23/26 2:24 PM, Krzysztof Kozlowski wrote:
All the functions operating on the 'handle' pointer are claiming it is a
pointer to const thus they should not modify the handle. In fact that's
a false statement, because first thing these functions do is drop the
cast to const with container_of:
struct ti_sci_info *info = handle_to_ti_sci_info(handle);
And with such cast the handle is easily writable with simple:
info->handle.version.abi_major = 0;
The const is for all the consumers drivers of the handle. Those
consumers cannot do the above becouse both handle_to_ti_sci_info()
and struct ti_sci_info itself are only defined inside ti_sci.c.
The code is not correct logically, either, because functions like
ti_sci_get_handle() and ti_sci_put_handle() are meant to modify the
handle reference counting, thus they must modify the handle.
The reference counting is handled outside of the ti_sci_handle struct,
the contents of the handle are never modified after it is created.
The const is only added by functions return a handle to consumers.
We cannot return non-const to consumer drivers or then they would
be able to modify the content without a compiler warning, which would
be a real problem.
This is the same argument as making pointer to const the pointer freed
via kfree() (or free() in userspace). kfree() does not modify the
contents of the pointer, right? The same as getting putting handle does
not modify the handle...
In that argument, if we wanted the consumer of the pointer to not free()
it we would return a const pointer, free()'ing that would result in the
warning we want (discards const qualifier).
If you could somehow malloc() from a const area in memory then free()
doesn't modify the pointed to values, only the non-const record keeping
which would be stored outside of the const memory. So even in this analogy
there isn't a problem.
The point is that storing the reference counter outside of handle does
not make the argument correct. Logically when you get a reference, you
increase the counter, so it is not a pointer to const. And the code
agrees, because you must drop the const.
The record keeping memory is not const and can be modified.
And where do we drop the const? The outer "struct ti_sci_info" was never
const to begin with, so no dropped const.
If the issue is that the handle is not const inside that outer struct
we could fix that,
struct ti_sci_info {
...
- struct ti_sci_handle handle;
+ const struct ti_sci_handle handle;
...
};
And with that change even your original commit message example issue
goes away,
struct ti_sci_info *info = handle_to_ti_sci_info(handle);
info->handle.version.abi_major = 0;
would now fail to work to compile.
Andrew
Andrew
Modification here happens anyway, even if the reference counting is
stored in the container which the handle is part of.
The code does not have actual visible bug, but incorrect 'const'
annotations could lead to incorrect compiler decisions.
Please kindly trim the replies from unnecessary context. It makes it
much easier to find new content.
Best regards,
Krzysztof
