Havoc Pennington hp at redhat.com
Tue Sep 12 08:08:30 PDT 2006


Peter Kümmel wrote:
> dbus_bool_t _dbus_open_file (DBusFile   *file,
>                              const char *filename,
>                              int         oflag,
>                              int         pmode);

small thing, if these are bitfields, should be unsigned int.

Are the flags really used though? this is just an ad hoc internal API, 
so it's fine to have e.g. dbus_create_file_exclusively() that creates, 
and dbus_open_file() that does not create, for example. we could add 
more as required. anyway, why not grep for "open" usages and see if 
there are patterns where only one or two sets of flags are ever used.

Higher level is better, it makes it easier to implement and keeps us 
from relying on detailed platform semantics.

> As intermidate step I've renamed _dbus_close to
> _dbus_close_not_a_file, and fixed the rest of the code.

Is there a not-a-file other than a socket? I think _dbus_close_socket is 
a bit clearer. (I thought the deal on windows was that there were 
"sockets" and "everything else" rather than "files" and "everything 
else"? but I don't really know)

> Should I use this type also for the unix code or
> is a other way preferred?

I think just putting the definition of the struct in the -unix.c and in 
-win.c makes sense, and make it the cleanest definition for each 
platform. If they are both just an int, that's fine, but it's not 
important to share a three-line struct definition across platforms. It 
is important to keep any of the outside-sysdeps code from cheating and 
accessing the int directly.

> And how should I prepare the patch, on patch for all
> _file, _socket, _pipe renaming?

The smallest patch that compiles and makes sense is usually best. If you 
wanted you could even separate the renaming (socket/file split) from 
adding the abstraction (DBusFile), though it isn't necessary I don't think.


More information about the dbus mailing list