Fix sizeof usage for Variant pointers in alloca
#84925
Conversation
|
I don't think them technically being the same matters here, having the actual type is important |
|
No sure how it matters with the sizeof usage as there is a clear cast as to the type they want it to be. |
|
Why does it not matter? It makes it clear what we want in case we change anything and need to update it, what's the problem with having the actual type for clarity? |
|
Sizeof should have an actual type, not a array of pointer type. Regardless its still clear that the allocation is for a list of pointers. |
|
My bad you're right it's taking the pointer type, this is correct, misread your description |
core/variant/callable_bind.cpp
Outdated
| @@ -133,7 +133,7 @@ void CallableCustomBind::get_bound_arguments(Vector<Variant> &r_arguments, int & | |||
| } | |||
|
|
|||
| void CallableCustomBind::call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const { | |||
| const Variant **args = (const Variant **)alloca(sizeof(const Variant **) * (binds.size() + p_argcount)); | |||
| const Variant **args = (const Variant **)alloca(sizeof(const Variant *) * (binds.size() + p_argcount)); | |||
There was a problem hiding this comment.
| const Variant **args = (const Variant **)alloca(sizeof(const Variant *) * (binds.size() + p_argcount)); | |
| const Variant **args = (const Variant **)alloca(sizeof(Variant *) * (binds.size() + p_argcount)); |
Minor suggestion to match the other instances, such as Object::callv
There was a problem hiding this comment.
Did you want it part of this request or other one?
There was a problem hiding this comment.
As this, were already fixing it 🙂
|
I would suggest squashing the commits, as they both fix the same issue in separate parts of the code. |
akien-mga
changed the title
alloca
akien-mga
changed the title
allocasizeof usage for Variant pointers in alloca
Coverity report this as a non portable usage of sizeof
alesliehughes
force-pushed
the
fix_sizeof_usage
branch
from
f05edbc to
5b6f641
Compare
|
Thanks! |
A few Coverity issue pickup with the use of sizeof, and in these cases are the same size.