Skip to content

Fix a number of bugs#227

Merged
sidorares merged 3 commits intosidorares:masterfrom
gcampax:master
May 31, 2018
Merged

Fix a number of bugs#227
sidorares merged 3 commits intosidorares:masterfrom
gcampax:master

Conversation

@gcampax
Copy link
Copy Markdown
Contributor

@gcampax gcampax commented Mar 23, 2018

These are bugs that I found over the course of using dbus-native.
I thought dbus-native was unmantained, but I'm seeing activity recently so I hope these can be merged so I don't need to keep the fork alive.

Each commit is a different bug.

gcampax added 3 commits March 23, 2018 14:15
All messages, including errors and replies, must have serials,
and serials must be unique and increasing.
A variant of a struct must have signature starting with '(' -
multiple values in a single variant is invalid. Conversely, a
variant of a single value can have a signature with length > 1,
if the type is a composite type.
In turn, that means the whole expression made no sense and should be killed.
Comment thread lib/bus.js
@@ -1,3 +1,5 @@
// -*- mode: js; c-basic-offset: 2; js-basic-offset: 2 -*-

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want modelines.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remove the modelines?

@gcampax
Copy link
Copy Markdown
Contributor Author

gcampax commented May 30, 2018

Other than the modeline thing, are there any other comments on the patch? Or any chance it will be merged eventually?

@sidorares
Copy link
Copy Markdown
Owner

looks good, sorry it was hanging for so long

@sidorares sidorares merged commit e190192 into sidorares:master May 31, 2018
@gcampax
Copy link
Copy Markdown
Contributor Author

gcampax commented May 31, 2018

Oh great, thank you!

@gcampax
Copy link
Copy Markdown
Contributor Author

gcampax commented May 31, 2018

I forgot to say, this PR should be a fix for #10, #9 and #143 (but without the tests in #143 )

@sidorares
Copy link
Copy Markdown
Owner

Can you confirm if it actually fixes issues and I'll close those?

@gcampax
Copy link
Copy Markdown
Contributor Author

gcampax commented May 31, 2018

It certainly fixes #10 and #9. I'm not completely sure about #143, but this PR fixes a bug in the area of variant marshalling so it is quite likely it's the same bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants