Created attachment 121363 [details] [review] proposed fix Due to a duplicate 'i++' in run_validity_tests in dbus-marshall-validate-util.c, only the even tests are run. With this fixed, we discover that the test for "a)" fails because it produces DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE instead of DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED. The attached patch fixes the bug and disables the failing test. <advertisement>Found by clang -Wfor-loop-analysis.</advertisement> ;)
Comment on attachment 121363 [details] [review] proposed fix Review of attachment 121363 [details] [review]: ----------------------------------------------------------------- ::: dbus/dbus-marshal-validate-util.c @@ -61,5 @@ > v, tests[i].data); > _dbus_assert_not_reached ("test failed"); > } > - > - ++i; Thanks, well caught. @@ +81,4 @@ > DBUS_INVALID_EXCEEDED_MAXIMUM_STRUCT_RECURSION }, > { ")", DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED }, > { "i)", DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED }, > + /* { "a)", DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED },*/ /* produces DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE */ This is not correct; commenting-out code is usually bad. If a test has accidentally not been run, and its result differs from what the code says it should be, the question to answer is: is the actual result acceptable? If yes, we should assert that the result is that (or perhaps that it is one of the reasonable options); if no, we should fix whatever is being tested. In this case, MISSING_ARRAY_ELEMENT_TYPE seems just as reasonable for "a)" as STRUCT_ENDED_BUT_NOT_STARTED - this string has both of those problems, it's just a matter of which one we hit first, and the author of this test happens to have picked the one not matching the implementation. So I'm going to uncomment this line and assert the real result.
Fixed in git for 1.10.8 and 1.11.2, thanks.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.