Bug 93908

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.