Passing file descriptors via dbus-cxx

[Nick Burkitt]

Am I correct that dbus-cxx in its present form does not support the passing of file descriptors in messages?

Thanks,

-Nick

[Robert Middleton]

Correct.

I've never had a need to do anything with that, and I've never encountered an interface that requires it.  I've never actually passed file descriptors around either, so I'm not even sure what the process is in the general case.

If you need to add this feature for some reason, I can point you in the right direction on what files would have to be modified and commit the new version.

-Robert Middleton

[Nick Burkitt]

Hi Robert.

It's pretty simple as a stand-alone operation. Here's a stackoverflow link. I have a kludge-around planned at the moment, but I'd much prefer simply to add a file descriptor to an object to be passed over the dbus. I don't promise any action before the end of the year, but I'd be happy to get some pointers.

Thanks,

-Nick

[Robert Middleton]

It should be as simple as what we talked about before, with defining a custom object.  The only difference in this case would be that it should be part of the library itself, not a custom type.

I could probably get it done pretty quickly if that is the case, but it will be a few days.  (the relevant classes should be signature, messageappenditerator, messageiterator)

-Robert Middleton

[Nick Burkitt]

If you're volunteering, I'm accepting! :-)

[Nick Burkitt]

Hi Robert.

It occurs to me that this may not require anything more than changing an 'i' to an 'h' in the signature (assuming the file descriptor is part of a custom data struct). The iterator insertion/extraction operations don't really care about type. All they should need to know is size - they only type information is contained in the signature. Since the file descriptor is really just an int, operator<<() and operator>>() for the struct can just get and put an int32_t. As long as the signature contains DBUS_TYPE_UNIX_FD ('h') in the appropriate position, what else is needed? 

Of course that won't support fds outside of custom structs, but then I don't intend to use them that way.

Or am I missing something obvious?

Thanks,

-Nick

[Robert Middleton]

That may partially work, but the problem is that when it is appended to the message it will have an 'int' type.

The relevant part of dbus that actually handles the conversion from a type to a DBus type: https://dbus.freedesktop.org/doc/api/html/dbus-message_8c_source.html#l02733

The relevant part of dbus-cxx: https://sourceforge.net/p/dbus-cxx/code/HEAD/tree/trunk/dbus-cxx/dbus-cxx/messageappenditerator.cpp#l77

Since the file descriptor is just an integer, it will be inserted into the message as an integer, so that's why a custom type within the library itself must be defined(there's no way to override this through user code).

-Robert Middleton

--
You received this message because you are subscribed to the Google Groups "Dbus-cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dbus-cxx+unsubscribe@googlegroups.com.
To post to this group, send email to dbus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/dbus-cxx/003a761a-84dd-47b9-9ee9-7fe810cd6f06%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Robert Middleton]

I should be able to take a look at it Thursday or Friday this week anyway.  

-Robert Middleton

[Nick Burkitt]

I tried experimenting with it and found that fd values were simply passed as-is to the receiver, as you might expect given your explanation. I also took a stab at adding the code to to the library to handle the type correctly, but with the same result. It would be great to have a better-informed version to try. :-)

I presume that if my copy of libdbus was compiled without HAVE_UNIX_FD_PASSING being defined, I'd see some sort of error, but I'll try to determine if that's the case anyway.

Thanks again for your help!

-Nick

-Nick

[Nick Burkitt]

I take it back - I had failed to add an extractor in messageiterator.cpp/h for my file descriptor type, and the extractor for my custom type was actually throwing a DBus::InvalidTypecast error. Once I corrected that oversight, it worked. I'm able to pass a file descriptor to another process. I ended with changes to

messageappenditerator.cpp - Added methods:

  bool MessageAppendIterator::protected_append( const FileDesc& v )

  {

    bool result;

    if ( not this->is_valid() ) return false;

    result = dbus_message_iter_append_basic( &m_cobj, TYPE_FILEDESC, &v.m_fd );

    if ( ! result ) m_message->invalidate();

    return result;

  }

  bool MessageAppendIterator::append( const FileDesc& v )

  {

    return this->protected_append( v );

  }

messageappenditerator.h - Added prototypes:

      bool protected_append( const FileDesc& v );

      bool append( const FileDesc& v );

messageiterator.cpp - Added method:

  FileDesc MessageIterator::get_filedesc()

  {

    FileDesc fd;

    if ( this->arg_type() != TYPE_FILEDESC )

      throw ErrorInvalidTypecast::create("MessageIterator: getting FileDesc and type is not TYPE_FILEDESC");

    dbus_message_iter_get_basic( &m_cobj, &fd );

    return fd;

  }

messageiterator.h - Added prototype:

      FileDesc    get_filedesc();

types.h - Added overload for type( FileDesc ):

  inline Type type( FileDesc )           { return TYPE_FILEDESC; }

enums.h - Added a value to enum Type:

    TYPE_FILEDESC = DBUS_TYPE_UNIX_FD,

signature.h - Added #include and overload for signature( FileDesc& )

#include <dbus-cxx/filedesc.h>

   inline std::string signature( FileDesc& )   { return DBUS_TYPE_UNIX_FD_AS_STRING;     }

And one new file,

filedesc.h - defines new FileDesc type:

struct FileDesc

{

int m_fd;

FileDesc()

{

m_fd = 0;

}

FileDesc( const int& fd )

{

m_fd = fd;

}

FileDesc& operator=( const int& fd )

{

m_fd = fd;

return *this;

}

int getValue( void ) const

{

return m_fd;

}

void setValue( const int& fd )

{

m_fd = fd;

}

};

Any comments or suggestions are more than welcome.

Thanks again!

-Nick

To unsubscribe from this group and stop receiving emails from it, send an email to dbus-cxx+u...@googlegroups.com.

To post to this group, send email to dbus...@googlegroups.com.

[Robert Middleton]

That's pretty much what I was going to do anyway. :)

If you can format a patch, that would be good.  There's a few changes that need to happen for it to be fully correct, so if you have a patch I can just build off of what you have, or you can fix them before sending the patch.

Things that need to be fixed:

• This is more of a stylistic thing, but FileDesc should be a class instead of a struct.  It should also be called FileDescriptor for clarity.
• 0 is a valid file descriptor, so that should be initialized to -1 in FileDesc
• FileDesc should also be in the DBus namespace
• The Makefile.am needs to be updated to install the new header
• The dbus-cxx master header needs to be updated to include the new header

-Robert Middleton

To unsubscribe from this group and stop receiving emails from it, send an email to dbus-cxx+unsubscribe@googlegroups.com.

To post to this group, send email to dbus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/dbus-cxx/34af57b2-73c8-43b1-b72b-067b5ec2bc88%40googlegroups.com.

[Nick Burkitt]

Hi Robert.

We both missed a hole in the file descriptor implementation. There's no support for FileDescriptor as as method return type.

When any of the MethodProxy::operator() methods is called, it executes this code:

      ReturnMessage::const_pointer retmsg = this->call( _callmsg );

      T_return _retval;

      retmsg >> _retval;

      return _retval;

Where T_return is FileDescriptor. At "retmsg >> _retval", MessageIterator::operator int32_t() throws a "casting non-numeric type to numeric value" exception, since it doesn't define a TYPE_UNIX_FD case. I'm not sure how you'd prefer things stylistically, but I worked around the problem by defining an operator MessageIterator::FileDescriptor() and adding a new case:

    case TYPE_UNIX_FD: return this->operator FileDescriptor().getDescriptor();

to all of the existing integer operator() definitions. The explicit syntax was necessary to disambiguate the reference from

DBus::FileDescriptor::FileDescriptor(int)

and

constexpr DBus::FileDescriptor::FileDescriptor(const DBus::FileDescriptor&)

While I was at it, I added FileDescriptor to the various string-izing methods in types.h.

Thanks,

-Nick

[Robert Middleton]

Good info, I'll take a look at it in a few days probably. There
should probably be a unit test to check for that as well; I was
considering adding it but I haven't gotten around to it. :(

-Robert Middleton

> --
> You received this message because you are subscribed to the Google Groups
> "Dbus-cxx" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to dbus-cxx+u...@googlegroups.com.
> To post to this group, send email to dbus...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/dbus-cxx/5eee8aa6-4539-464f-9858-291c01bd4567%40googlegroups.com.

[Robert Middleton]

I've just committed a (possible) fix for this; I haven't fully tested
it out yet on my side to ensure that it is actually working
completely. I added the appropriate cast to the messageiterator, and
also added a unit test to ensure that it is working properly. The
reading/writing from a file descriptor seems to work fine, but I need
to test it between processes.

I did remove the explicit FileDescriptor(int) constructor to get
around the ambiguity, although now that I'm re-reading your email I
now see how you got around that. Any preferences on if that
constructor should be kept?

-Robert Middleton

[Robert Middleton]

I took another look at it, and figured out another possible way to
create the FileDescriptor objects. It follows the same pattern of
most of the other dbus-cxx objects, where there is a static create()
method in the file descriptor class that you use to construct it.
This gets around the problem of the constuctor(adding explicit didn't
help at all).

I'm flexible doing this any way, but this seems to match other code.
The downside is that this means that when passing the file descriptor
back and forth across the bus, that is the only type that gets passed
as a pointer. All of the other objects(e.g. ints, strings, etc) are
passed as a non-pointer.

I also started on an example program(it's under
examples/basics/filedescriptors), but it's not passing the descriptor
properly. It looks like the server is not passing back the signature
properly; an error comes back across the bus.

-Robert Middleton

[Robert Middleton]

I fixed the file descriptor passing, so that example program now
works(typo in the code). The current implementation works now, so
does the API as currently written make sense?

If it does, I'll cut a new release in about 2-3 weeks to give some
time for any other bugfixes.

-Robert Middleton

[Nick Burkitt]

Hi Robert.

Sorry - I haven't had a chance to look at this at all. I have a deadline looming next week, so...probably after that.

-Nick

-Nick

>>>> email to dbus-cxx+unsubscribe@googlegroups.com.

[Nick Burkitt]

Hi Robert.

I finally had some time to look at this. How about this as the FileDescriptor class:

class FileDescriptor

{

public:

FileDescriptor() : m_fd( -1 ){

}

static FileDescriptor create( const int& fd ){

FileDescriptor _fd;

_fd.m_fd = fd;

return _fd;

}

int getDescriptor() const{

return m_fd;

}

private:

int m_fd;

};

That way there's no strangeness with shared pointers, and no constructor that accepts an integer argument to confuse anything. If you don't like having a "create" method that doesn't return a pointer, you can always call it something else.

I just updated to SVN rev. 241. I've linked it as a static library to facilitate debugging. I hope to give it a good workout over the next week or two to see if it breaks. If it does, I'll try to track down the root cause.

-Nick

-Nick

>>>> email to dbus-cxx+u...@googlegroups.com.

[Robert Middleton]

That could work too, I'll have to take a closer look at it in the
coming days. For now at least, it's currently using the shared
pointers.

Let me know how it works out for you; it's working good for me at the
moment, but I'm certainly not exercising as much as you.

-Robert Middleton

> https://groups.google.com/d/msgid/dbus-cxx/3e3087a1-0450-40c0-bd75-1725144c5225%40googlegroups.com.

[Robert Middleton]

Have you had any problems with the current code?  I'm planning on doing a release in a few days if everything goes well.

I may change the FileDescriptor class as well to do it your way instead of the shared pointer way, but I don't want to change it if it's going to break something on your end.

-Robert Middleton

> email to dbus-cxx+unsubscribe@googlegroups.com.

[Nick Burkitt]

Hi Robert.

I haven't yet found an opportunity to really try to break things, but we have had very few problems with the latest code. 

I'm currently building the library from source and statically linking to my project(s), so I won't be affected by any changes until I update from SVN. Either way, though, the interface changes are minor to nonexistent, so don't let that concern sway you one way or the other.

When I finally get a chance to really stress it, I'll let you know what I find. Darned customers keep interfering. :-/

Thanks,

-Nick

-Nick