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

Async API review #1

Draft
wants to merge 24 commits into
base: master
Choose a base branch
from
Draft

Async API review #1

wants to merge 24 commits into from

Conversation

d12fk
Copy link
Owner

@d12fk d12fk commented Apr 14, 2022

This PR is for reviewing the changes, and also to get notified when new stuff is force pushed.

d12fk added 2 commits April 14, 2022 13:29
  * remove quotes from name (not needed)
  * while at it rephrase
  * remove empty Requires

Signed-off-by: Heiko Hund <[email protected]>
@d12fk d12fk marked this pull request as draft April 14, 2022 12:34
@d12fk d12fk self-assigned this Apr 14, 2022
@d12fk d12fk force-pushed the async_api branch 4 times, most recently from 7403650 to 0e4bdec Compare April 15, 2022 00:48
src/dbus_names.h Outdated
@@ -36,14 +36,6 @@ namespace DBus {
enum Type {
BusName, UniqueName, WellKnownName, ErrorName,
InterfaceName, NamespaceName, MemberName };
static constexpr std::array<const char*, 7> nameStr = {
Copy link
Owner Author

Choose a reason for hiding this comment

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

@mikecrowe this symbol was missing according to clang, has this to do with -fvisibility as well? If so what's happening?
Moving the symbol to the .cpp made it link again

Choose a reason for hiding this comment

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

This might be similar to 7ca1104.

I must admit that I'm not at all expert with -fvisibility, especially with C++, and the need to use -fuse-ld=gold in ff563e1 doesn't inspire confidence that all is well. Does the problem go away if you revert ff563e1?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Reverting didn't help: https://github.com/d12fk/dbus-asio/runs/6042722273
I have no idea what clang doesn't like here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This works with -std=c++17, so I guess there's an issue with std::array that's resolved in later a std. For now I would like to keep it to -std=c++1y, as RHEL7 is that far back and supported for another year.

Choose a reason for hiding this comment

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

RHEL7 is that far back and supported for another year.

RHEL7 is supported until end of June 2024, so two more years. It by default ships with GCC 4.8.5 (IIRC), which only supports -std=c++1y (which is most of, but not all, C++14 features). That said, the devtoolset-10 software library is available, which gives GCC-10.3, with should support -std=c++17. OpenVPN 3 Linux is already being built with this compiler for the official releases.

@d12fk d12fk force-pushed the async_api branch 2 times, most recently from 7d78237 to 0e4bdec Compare April 15, 2022 20:53
d12fk added 2 commits April 19, 2022 13:19
This set of types makes it possible to validate strings according to the
requirements for certain "names" as specified in the DBus spec. The sections
"Valid Names", "Valid Object Paths" and "Match Rules" in the spec define the
rules for validaton. The types are:

  * UniqueName - a unique connection name
  * WellKnownName - a unique name alias, aka well-known bus name
  * BusName - either one of the previous two, a "bus address"
  * ObjectPath - a object path as used in the message header
  * InterfaceName - a interface name as used in the message header
  * MemberName - a member name as used in the message header
  * ErrorName - a error name as used in the message header
  * NamespaceName - a special kind of bus name used in match rules

The types are opaque, since they take a C or std:: string and have
an operator std::string().

Signed-off-by: Heiko Hund <[email protected]>
Change the way MatchRules are constructed. Instead of taking a string
and then parsing it, take the match keys in a structured way, validate
them and generate a match rule string from them.

Added support for missing match rules keys, except "eavesdrop", since that
one is deprecated in favor of the "BecomeMonitor" method.

Signed-off-by: Heiko Hund <[email protected]>
@d12fk d12fk force-pushed the async_api branch 4 times, most recently from d2827d4 to 79ea89a Compare April 23, 2022 09:52
d12fk added 10 commits April 23, 2022 12:18
Let the user decide which version of ASIO to build against. Users of
dbus-asio need to link against the same version of asio, so this adds
some flexibility if they can't or don't want to switch to boost.

Signed-off-by: Heiko Hund <[email protected]>
This error abstraction is able to hold either a error_code as used
by asio, or a DBus::Message::Error as received from dbus.

Signed-off-by: Heiko Hund <[email protected]>
Also added some spacing after 8 bytes to make things look prettier.
Output looks like this now (hex shortened):

    6c ... 00  02 ... 00  06 ... 00 | l....... ........ ..s.....
    6f ... 65  64 ... 2e  44 ... 00 | org.free desktop. DBus....
    01 ... 00  2f ... 65  65 ... 70 | ..o..... /org/fre edesktop
    2f ... 00  02 ... 00  6f ... 65 | /DBus... ..s..... org.free
    64 ... 2e  44 ... 00  03 ... 00 | desktop. DBus.... ..s.....
    52 ... 4e  61 ... 00  08 ... 00 | RequestN ame..... ..g..su.
    0a ... 74  2e ... 00  00 ...    | ....test .steev.. ....

So, you can tell pretty good what's going on in the message blob without
dissecting the bits carefully.

Signed-off-by: Heiko Hund <[email protected]>
The previous approach using boost::any as the generic DBus type had the
downside that any data(type) could be fed into it, no matter if DBus
actually supports the datatype.

Instead, there's now a DBus::Type::Any which behaves more like a
std::variant with some DBus specific extras. It serves as the container
for the others inheriting from DBus::Type and as a generic type the
DBus container types can store.

This type system is more type safe than before, so there's no need to
compare types by their serialized string form, as each can be cast back
to the native C++ type.

The Signature type now validates the signature string passed to it. So,
it is no longer possible to use DictEntry's outside of an Array, like
the standard mandates.

The ObjectPath type makes use of the DBus::ObjectPath "name" and the
validation done therein.

The Array type checks the all Array::add()ed elements have the correct
signature and/or type.

Made the DBus::Type::toString() output a bit more compact and reflect
the container types a bit better by using [], {} and () in the right
places.

Signed-off-by: Heiko Hund <[email protected]>
DBus has support for passing file descriptors between processes via
the Unix sockets. This adds the respective type and prepares buffer and
streams to handle such FDs when sending and receiving messages.

Signed-off-by: Heiko Hund <[email protected]>
Most of things are now done in the the base type DBus::Message (was:
DBus::Message::Base). As a consequence the types representing the four
DBus messages hardly have any code left and their implementation was
moved in with the rest of the message handling code.

The DBus::Message::Header now makes use of the DBus type system and has
support for all header fields. It also got a toString() for logging.

Same goes for the body, a.k.a parameters, which can now be logged for
incoming and outgoing messages using the DBus::Type system.

The MethodCallIdentifier and MethodCallParameters types are now simply
known as Identifier and Paramaeters, since they are used with other
messages than MethodCall as well. MethodCallParametersIn is now the
non-const part of the Parameters type.

Signed-off-by: Heiko Hund <[email protected]>
Convert to use functionality from the new DBus::Type system.

Signed-off-by: Heiko Hund <[email protected]>
There's specialized functions for sending authentication protocol
commands and DBus messages. Messages can transport file descriptors as a
side load in cmsg'es. All is pretty straight forward (if you also consider
cmsg that).

Signed-off-by: Heiko Hund <[email protected]>
The class now implements the complete state machine for the DBus
authentication protocol and handles all commands.
It's works completely async.

With little work it should be doable to subclass different auth protocols
from this. However, I'm not sure if there's need for more than what has
been there before, so I left it as it is for now.

Signed-off-by: Heiko Hund <[email protected]>
Sending and receiving of messages is now done asyncronously.

In the receive loop, we first peek at the message to determine its size
and the receive it in one go including file descriptors, potentially.
Received messages are dispatched to their registered handlers, a.k.a
completion tokens, to be dealt with.

Added the DBus::StoredToken helper class to keep a hold of the callers
completion token while we're waiting for a message event it receives
to happen, as ASIO itself currently does not have support for that.

Signed-off-by: Heiko Hund <[email protected]>
@d12fk d12fk force-pushed the async_api branch 2 times, most recently from cd0fff4 to 30d47b3 Compare April 23, 2022 10:52
@d12fk d12fk force-pushed the async_api branch 6 times, most recently from b0639be to 3b413f9 Compare April 23, 2022 11:39
d12fk added 10 commits May 16, 2022 15:47
This is the class to tie all things together. It replaces DBus::Native
with pretty much comparable functionality. Just think the name suits
better.

Offers an interface to connect to DBus and deal with messages on a low
level. There are also helper functions for DBus interfaces and messages.

Signed-off-by: Heiko Hund <[email protected]>
They have been converted 1:1 even if the functionality doesn't make much
sense anymore. To be extended at a later time.

Signed-off-by: Heiko Hund <[email protected]>
Get rid of boost and pthreads.

Signed-off-by: Heiko Hund <[email protected]>
BusName, InterfaceName and ObjectPath are not needed in all messages and
thus can be left undefined for those. This can be achieved by default
constructing them.

Added an operator bool() to test for this. Also support ostream'ing
names via operator const char*().

Signed-off-by: Heiko Hund <[email protected]>
Since each represents one flag value, it's a bit easier on the eye.

Signed-off-by: Heiko Hund <[email protected]>
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.

3 participants