[patch] system activation 3

Havoc Pennington hp at redhat.com
Thu Jun 28 11:28:58 PDT 2007


Hi,

Richard Hughes wrote:
>>>  INCLUDES=-I$(top_srcdir) $(DBUS_BUS_CFLAGS)				\
>>> +	-DDBUS_SYSTEM_CONFIG_FILE=\""$(configdir)/system.conf"\"	\
>> The way this is done for session.conf recently changed; the reason to 
>> change it for session.conf (portability) doesn't really apply to Windows 
>> which has no system bus, but perhaps for symmetry the system bus should 
>> be done the same way.
> 
> I don't follow - we need the define for the launch helper.

I was talking about the function in dbus-sysdeps to get the session 
config file, which replaced the previous #define.

Don't worry about it ;-) it doesn't really make a difference.

> We already do _dbus_clearenv, surely this would nuke PATH?

Yes, sorry, I had some idea in my head that it wasn't called until 
partway through the helper code, but it's called at the start.

> Sure, I've refactored this file pretty heavily (attached, would
> appreciate new preliminary review) and added the OOM check. I'm still
> fixing some OOM failures (arrh!!) but I'll be finished soon. I'm not
> sure how to set TEST_OOM_CHECKS either.

I think the easiest approach to this is to build two binaries; IOW split 
into three files:
  - production main()
  - a main() that runs in 'make check' (just a binary in TESTS)
  - run_launch_helper()

You could also put any other assorted random unit tests in the 'make 
check' binary, if that's useful.

You could just stick the 'make check' stuff in the existing bus-test 
binary, except for the conflicting symbols for the two XML parsers, 
which ends up requiring a separate binary I would think.

>>> +                    sitter->errnum = WEXITSTATUS (sitter->status);
>> Again in the nitpick category, errnum is an odd name for an exit status 
>> variable.
> 
> Hmm, I reused it to avoid refactoring large chunks of code. I'll see
> what I can do.

Probably I didn't fully grok what was going on here. Use your judgment, 
don't do anything that doesn't make sense.

Havoc



More information about the dbus mailing list