[patch] make dbus-launch export vars, usage bits

Havoc Pennington hp@redhat.com
Fri, 16 May 2003 14:51:36 -0400


Hi,

On Fri, May 16, 2003 at 01:16:37PM -0400, Colin Walters wrote:
> Hi,
> 
> The attached patch makes dbus-launch export the variables it prints.  I
> made it able to emit either Bourne syntax or csh syntax, like q-agent
> and ssh-agent.  This is so you can just say:
> 
> eval `dbus-launch` in your ~/.xsession or whatever.
> 
> Also, I know this is minor, but I think when you pass --help to a
> program, it shouldn't exit with an error.  So I fixed dbus-send.
> 
> Ok to commit?

Looks good, one big-picture comment: I think the default should remain
as it is now, and the full shell code should only be used if you
specify one of the -s or -c options. The reason is that we might want
to use dbus-launch programmatically without a shell, and the KEY=VALUE
syntax is pretty easy for anyone to parse. 

Also, we may well want to have some variables that aren't worth
exporting (are just there for use in the actual script that does the
launching). The PID variable is already like that, really.  In fact
you might want to change the patch so it doesn't export the pid, only
the address.

I guess this suggestion comes down to: default to the current simple
KEY=VALUE output, and add --auto-shell-syntax or something that does
what your current default does (assuming we'll ever be run from an
unknown shell - will that happen?).

 static void
-usage (void)
+usage (int ecode)
 {
-  fprintf (stderr, "dbus-launch [--version] [--exit-with-session]\n");
-  exit (1);
+  fprintf (stderr, "dbus-launch [--version] [--help] [-s] [-c] [--exit-with-session]\n");

I'd tend to use the long options in usage() (I'd almost say the short
options shouldn't exist but they are probably harmless; still, having
them in usage() isn't very helpful, while long options might be
self-explaining).


+      else if (strcmp (arg, "-c") == 0
+	       || strcmp (arg, "--csh-syntax") == 0)

D-BUS style would put the "||" on the previous line.


+	  if (!strncmp (shname + strlen(shname) -3, "csh", 3))

D-BUS style would put a space before the "(" after strlen.

+      if (bourne_shell_syntax)
+	{

I think this brace would be indented one more column in GNU style.

-.B dbus-launch [\-\-version] [\-\-exit-with-session]
+.B dbus-launch [\-\-version] [\-c] [\-s] [\-\-exit-with-session]

Man page should include the long options as well.

Havoc