Bug 13305

Created attachment 12630 [details]
client listening for events

Created attachment 12631 [details]
server broadcasting events

ok, after following your code I do see the off by one error.  Thanks for the test case.  I need to check if the semantics are different with arrays present but otherwise it should be an easy fix.

Is the bug here just that the error message has the wrong number in it?

Yes, it is simply the error message though there is code that resets i and reuses it as a counter when an array is seen so it may be completely broken in that case.  I'm investigating.

If it's too complicated to properly fix, then at least change the error message to _not_ print the number, but instead just say 'message has less arguments than expected'. But if you print a number that is wrong it will confuse the users (well, it did confuse me...).

yep that array counter is wrong too.  Sending in a 4 element array instead of
the string gives me the error message:

Message parsing error: Message has only 4 arguments, but more were expected

Patch coming

Created attachment 12653 [details] [review]
Patch to fix the off by one error message

This should do it

The patch contains these two lines:

              j = 0;
              while (i < n_elements)

That looks unlikely to be correct? (i/j mismatch)

Can't the array-iteration variable be local to the block that does the array iteration, reducing the potential for bugs?

Even nicer, maybe break the array part out into a separate function (passing it a "va_list*" probably)

woops, went off to other things and forgot about this one.  I'll have to rewrite the patch so it isn't going to make the next release.  I'll make sure to do something for 1.2.0 release though. Moving this to high priority just to keep it at the top of my queue.

(In reply to comment #10)
> woops, went off to other things and forgot about this one.

Ping?

Comment on attachment 12653 [details] [review]
Patch to fix the off by one error message

Review of attachment 12653 [details] [review]:
-----------------------------------------------------------------

I'll upload a new version based on J5's patch soon.

::: dbus/dbus-message.c
@@ +754,1 @@
>                while (i < n_elements)

fail, should be j here.

@@ +758,3 @@
>                                                  &s);
>                    
>                    str_array[i] = _dbus_strdup (s);

fail, should be j here.

Created attachment 86791 [details] [review]
[PATCH] fix off by one error message (#13305)

This is basically cosmetic (i and j are only used in error messages) so I'm not going to put this in the 1.6 stable branch. Fixed in git for 1.7.6.