[PATCH] xfree86: rename Xorg.bin to Xorg

[Peter Hutterer]

On Thu, Dec 18, 2014 at 02:55:07PM -0800, Keith Packard wrote:
> Peter Hutterer <peter.hutterer at who-t.net> writes:
> 
> > If the suid wrapper is enabled, /usr/bin/Xorg is just a shell script that
> > execs either /usr/libexec/Xorg.bin directly or the Xorg.wrap binary which then
> > execve's /usr/libexec/Xorg.bin.
> >
> > Either way, we end up with Xorg.bin, which is problematic for two reasons:
> > * ps shows the command as Xorg.bin
> > * _COMM and _EXE in systemd's journal will both show Xorg.bin as well
> >
> > There's not much we can do about the path, but having the actual command stay
> > as Xorg means better compatibility to existing scripts. And, the reason for
> > this patch: the command
> >    journalctl _COMM=Xorg
> > works universally, regardless of whether the wrapper is used or not.
> 
> Seems like a sensible change to me
> 
> > diff --git a/hw/xfree86/xorg-wrapper.c b/hw/xfree86/xorg-wrapper.c
> > index 4ea4733..9ae8dec 100644
> > --- a/hw/xfree86/xorg-wrapper.c
> > +++ b/hw/xfree86/xorg-wrapper.c
> > @@ -255,18 +255,18 @@ int main(int argc, char *argv[])
> >          }
> >      }
> >  
> > -    snprintf(buf, sizeof(buf), "%s/Xorg.bin", SUID_WRAPPER_DIR);
> > +    snprintf(buf, sizeof(buf), "%s/Xorg", SUID_WRAPPER_DIR);
> >  
> >      /* Check if the server is executable by our real uid */
> >      if (access(buf, X_OK) != 0) {
> > -        fprintf(stderr, "%s: Missing execute permissions for %s/Xorg.bin: %s\n",
> > +        fprintf(stderr, "%s: Missing execute permissions for %s/Xorg: %s\n",
> >              progname, SUID_WRAPPER_DIR, strerror(errno));
> >          exit(1);
> >      }
> >  
> >      argv[0] = buf;
> >      (void) execv(argv[0], argv);
> > -    fprintf(stderr, "%s: Failed to execute %s/Xorg.bin: %s\n",
> > +    fprintf(stderr, "%s: Failed to execute %s/Xorg: %s\n",
> >          progname, SUID_WRAPPER_DIR, strerror(errno));
> >      exit(1);
> 
> This could be simplified to just use the computed name in the two
> locations that print it out?

true, both changed locally

> Otherwise, this change is reviewed for technical correctness, but not
> for whether it makes sense for distributions :-)
> 
> Reviewed-by: Keith Packard <keithp at keithp.com>

thanks,

Cheers,
   Peter