GNOME Bugzilla – Bug 611529
functions with out arguments have wrong argument indexing
Last modified: 2010-03-01 22:47:08 UTC
Functions with "out" arguments lead to an of by one error in the arguments following arguments. I believe the problem is that the index pointing to the arguments passed to JavaScript is incremented even if the function argument is 'out', though in that case the argument should not have been passed to JS. We found this out with g_file_replace_contents, where the cancellable argument gets passed a value at argv[argc].
Created attachment 154980 [details] [review] gi: Do not increment argument counter for OUT arguments OUT arguments are not passed in JS, so do not increment the index in the argv array when dealing with an OUT argument.
Review of attachment 154980 [details] [review]: Looks correct
Ideally we should add a test here, can you do that Johan?
(In reply to comment #3) > Ideally we should add a test here, can you do that Johan? Will do
Review of attachment 154980 [details] [review]: ::: gi/function.c @@ +632,1 @@ ++in_args_pos; shouldn't this be moved as well? (based on name it has similar semantics and is relevant only for in arguments...)
Also, maybe adding asserts for argv_pos < argc and in_args_pos and friends might be in order so that suchs out of bounds accesses don't go silently unnoticed.
Created attachment 154981 [details] [review] Add a test function with an out argument and some other after that Useful to debug case
Created attachment 154982 [details] [review] test: Add a (failing) test with an "out" argument followed by another
Created attachment 154983 [details] [review] gi: Do not increment argument counter for OUT arguments OUT arguments are not passed in JS, so do not increment the index in the argv array when dealing with an OUT argument.
(In reply to comment #5) > Review of attachment 154980 [details] [review]: > > ::: gi/function.c > @@ +632,1 @@ > ++in_args_pos; > > shouldn't this be moved as well? (based on name it has similar semantics and is > relevant only for in arguments...) in_args_pos seems to be an index in in_arg_cvalues, which has one item per argument to the C function, not the JS one. So it seems correct to increment it even for 'out' arguments