win32 patch dbus_watch_get_handle patch

Havoc Pennington hp at redhat.com
Sat Jun 16 22:58:19 PDT 2007


Hi,

Ralf Habacker wrote:
> So why  should this be an important porting task ? 
>

1) DBusWatch is a public API, so it should expose what Windows 
programmers expect, which is native Windows stuff.

2) emulation hacks keep leaving code broken, because they make things 
build and "work" when said things don't make any sense.  We went through 
all this with the dbus_uid_t fiasco.

3) DBusWatch should be API-consistent with DBusConnection in the public 
API, and internally it should maintain the same sockets vs. other fds 
approach we are using everywhere else

The thing to copy from DBusConnection is the get_socket() vs. 
get_unix_fd() approach.

4) the code does in fact rely on only sockets being returned, e.g. the 
watch socket is used in dbus-mainloop.c with _dbus_poll which only works 
on sockets on Windows.

If you want to fix dbus-mainloop.c for now by doing

#ifdef DBUS_WIN
get_socket()
#else
get_fd()
#endif

then that's fine, but we need to get the DBusWatch API correct because 
it's public.

5) This task should take all of 30 minutes to do cleanly and properly, 
which is less time than it has taken so far to file bugs, post to the 
list, etc.

> Is DBusFile intended as cross-platform file io class ?  This would be 
> understandable for me. whatever the win32 implementation either c 
> runtime or win32 api DBusFile would be the public class.

No, I don't want any DBusFile in the public API. That is not the job of 
libdbus; that is the job of GLib/gvfs, Qt, Java, Python, and so forth. 
Nobody is going to write an app that uses DBusFile as its file 
representation.

What we need to accomplish in libdbus is that GLib and Qt can easily get 
the native file or socket or pipe object and know what it is exactly in 
native terms. Then the GLib, Qt, Python, Java, etc. bindings can 
encapsulate what libdbus returns in their normal I/O classes.

Last time I checked, neither GLib, Qt, Python, Java, or any other 
application framework had a file or socket object with a constructor 
taking a DBusFile argument.

> Then i suggest to define a new functions  dbus_watch_get_unix_fd() 
> beside the given one and mark dbus_watch_get_fd() as deprecated.

This would be worth considering, but let's do it in a separate patch 
after we sort out the rest of the changes to DBusWatch.

> Why not 
> defining a DBUS_DEPRECATED which let's the compiler print a warning if 
> such a function is used.

We already have this, see dbus_message_iter_get_array_len() for example.

> What hinders you to do so for the unix part ? 

There should not be any unix work to be done to speak of here, that I 
see. But even if there is, I never volunteered to do the Windows port, 
or the cross-platform refactoring to make the Windows port possible 
(said refactoring is virtually all the work). I didn't volunteer because 
I don't have time. I did a few patches this week since I needed them for 
other reasons.

I did do DBusConnection get_unix_fd()/get_socket(), and the docs I wrote 
for those pretty much explain how DBusWatch should work as well, so the 
patch we're talking about is to just copy that.

If you do the one-line #ifdef I suggested above to make dbus-mainloop.c 
work I can live with it for now. Given that #ifdef, there should not be 
anything hard about doing DBusWatch as I've described.

In fact the changes are probably:
  - make get_fd() return -1 on Windows - just use an #ifdef hack for now,
    as dbus_connection_get_unix_fd does
  - add get_socket() that returns watch->fd
  - fix callers of get_fd() that need to work on Windows to use
    get_socket() on Windows - #ifdef it for now if necessary

We should clean it up later so DBusWatch knows whether it has a socket 
or what, but for now the #ifdef will get us by.

Havoc



More information about the dbus mailing list