-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mono] CodeQL fix set #1 #99637
[mono] CodeQL fix set #1 #99637
Conversation
icomparable_inst = mono_class_inflate_generic_class_checked (icomparable, &ctx, error); | ||
mono_error_assert_ok (error); /* FIXME don't swallow the error */ | ||
g_assert (icomparable_inst); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wish we could teach CodeQL that for some functions that have a MonoError *
argument, that if is_ok (error)
then the return value is non-null. Otherwise these kinds of assertions are pretty lame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, but I wish we could systematically fix these code patterns:
SomeResult *result = callee(arg1, arg, error);
mono_error_assert_ok (error); // we should get to know that result != NULL here
by adding some kind of annotation to callee
.
Yeah, I'm not sure whether there is no relevant annotation or if the annotations just aren't documented. |
CodeQL flagged various places where we're dereferencing pointers that could be NULL, this PR systematically cleans some of them up via g_assert. (Note that CodeQL doesn't currently recognize g_assert as an assertion... working on that)