hp at redhat.com
Tue Jun 27 20:41:55 PDT 2006
Peter Kümmel wrote:
> Seems Tor already has implemented a memory management:
> the strings are cached, and freed at shutdown.
> So we could your approach 2. "true value objects", or is
> it more the 3. way?
With the hash table, 2 is possible; I think 1 or 3 would be slightly
better but I'd be OK with a patch for 2. I would like to avoid the
#ifdef's in the header file, though ... the type should be opaque,
whether it's a pointer or an integer.
If you do 2, I guess the simplest option could be just leaving the
dbus_uid_t as an integer, and have uid_to_sid and sid_to_uid functions
using the hash table.
Or perhaps to go more cross platform, you could have a "uid to string"
and "uid from string" function ? where the string would be an "external
form" and the integer is not allowed to leave the dbus process, on
> I'm not 100% sure how I should proceed. I've here a
> 200k diff against HEAD and some new files.
I would do things as follows:
- check out a clean source tree
- one at a time in logical units, merge parts of the patches into the
clean tree, post for discussion, and get them checked in. If you use the
clean opaque data types approach, it should be possible to commit
patches one at a time without breaking anything
- you may need to first merge the build-related stuff if any so you
can compile a nonworking source tree on Windows
If you need to just "comment out" stuff on Windows at first that's OK:
Then in config.h:
I'd rather avoid using _WIN32 directly for temporary commenting out, so
it's easy to find the temporary fixes later, no?
You might also check out multiple source trees and get a couple of
patches "in flight" at once if you want.
In general I'm 100% OK with large changes and with committing things
piecemeal (in logical units), but I consider it important to keep things
maintainable and be sure the windows tree won't be accidentally broken
all the time by unix developers. One good route to this is to be sure
the headers (and semantics of internal/external APIs) are as identical
as possible on all platforms.
I'd want any changes to the core code to be pretty clean and in
consistent style with the rest of the source tree.
The windows-only files I'm not as picky about.
Windows-specific features are just fine too, they are better than hacks;
if we can't build a good cross-platform abstraction just be sure
"windows" or "win32" is in the name of the API. Or e.g. we could have
config file options that are Windows-specific.
For example, I'd rather have config file support for a native windows
logical IPC method, instead of overloading the UNIX socket stuff on
Windows when it doesn't make any sense.
> Should I just post the patches related to the changes to
> the original types?
> Does it help when I post the diff and new files?
I find it helpful if you post the whole logically-related change, at
least the first time you post a particular patch. If you post multiple
versions of a patch just the "interesting part" can go in later posts.
>> _dbus_uid_init_unix(DBusUID *uid, unsigned long value)
>> DBusUIDReal *real = (DBusUIDReal*) uid;
>> _dbus_assert(sizeof(DBusUIDReal) <= sizeof(DBusUID));
>> real->uid.unix = value;
>> That would involve relatively few changes to code, though I'd still
>> rather pass it in to functions as "DBusUID *uid" and not "DBusUID uid"
>> to match the usual pattern.
> I don't see the advantage of this approach, because we have to change
> much code to introduce DBusUID so it does not care how we change it.
> We could also pass pointers using my approach.
> And isn't it better to have a static type checking instead of the
> dynamic sizeof check?
I think doing it the same way as the rest of the codebase is probably a
good enough reason, whether the rest of the codebase is absolutely the
best way or not... makes it a lot easier to learn a codebase if it has
Approaches 1 and 3 are always cleaner than 2, but if you look at
DBusMessageIter, DBusString, etc. dbus does at least have a consistent
approach to 2.
You could also just go with the "keep the uid as integer on all
platforms" approach which is probably fine.
>> btw, what about guid_t? What's up with that on Windows, should it get a
>> parallel/identical treatment?
> I first wanna see what happens when DBusUID is introduced and how the
> merge process will work.
OK, but we should be sure the changes are parallel - we don't want to do
these two in two different ways.
More information about the dbus