Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a number of bugs #227

Merged
merged 3 commits into from
May 31, 2018
Merged

Fix a number of bugs #227

merged 3 commits into from
May 31, 2018

Conversation

gcampax
Copy link
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.
@@ -1,3 +1,5 @@
// -*- mode: js; c-basic-offset: 2; js-basic-offset: 2 -*-

Copy link

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

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
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
Owner

looks good, sorry it was hanging for so long

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

gcampax commented May 31, 2018

Oh great, thank you!

@gcampax
Copy link
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
Owner

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

@gcampax
Copy link
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