GNOME Bugzilla – Bug 611603
add more assertions to gjs_invoke_c_function
Last modified: 2016-11-29 07:33:28 UTC
Mainly for reassurance that we loop over the multiple arguments arrays as expected, not going out of bounds or leaving anything out.
Created attachment 155040 [details] [review] gi: make it more clear only processed_in_args are being released If the loop ends "prematurely" because of processed_in_args then the invoke had failed and INOUT/OUT arguments wouldn't be handled anyway so there's no point in continuing with the loop.
Created attachment 155041 [details] [review] gi: assert we don't access argv out of bounds Helps avoid bugs that might otherwise silently creep in.
Created attachment 155042 [details] [review] gi: assert all arguments are processed prior to invoking the function
Created attachment 155043 [details] [review] gi: assert all arguments were released after invoking the function Or at least assert the array indexes are as expected after iterating through the arguments.
Review of attachment 155043 [details] [review]: Yes
Review of attachment 155042 [details] [review]: Yes
Review of attachment 155041 [details] [review]: Yes
Review of attachment 155040 [details] [review]: Yes
Attachment 155040 [details] pushed as c37384d - gi: make it more clear only processed_in_args are being released Attachment 155041 [details] pushed as 29bc446 - gi: assert we don't access argv out of bounds Attachment 155042 [details] pushed as 2a5dbdb - gi: assert all arguments are processed prior to invoking the function Attachment 155043 [details] pushed as 7d572c6 - gi: assert all arguments were released after invoking the function Any reason for leaving the bug open?
No reason, thanks for noticing.
Created attachment 156428 [details] [review] remove incorrect asserts in function.c For example Gdk.Screen.get_default().get_root_window().get_pointer can return unknown mask(It will throw error arg.c:56). It is a normal situation.
Review of attachment 156428 [details] [review]: This is good, though could use a test that would trigger the assertion. A call with incorrect argument seems to do the trick which was not intended. I would also prefer some assertions also for the error cases to make sure we dealt with all the arguments as we're supposed to.
All patches seem to have been committed.