[PATCH v2 2/2] xf86LogInit: log to XDG_DATA_HOME when not running as root

[Hans de Goede]

Hi,

On 04/01/2014 09:33 AM, Peter Hutterer wrote:
> On Wed, Mar 26, 2014 at 12:24:50PM +0100, Hans de Goede wrote:
>> When no logfile was specified (xf86LogFileFrom == X_DEFAULT) and we're not
>> running as root log to $XDG_DATA_HOME/xorg/Xorg.#.log as Xorg won't be able to
>> log to the default /var/log/... when it is not running as root.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  configure.ac                   |  4 ++++
>>  hw/xfree86/common/xf86Helper.c | 18 +++++++++++++++++-
>>  hw/xfree86/man/Xorg.man        |  6 ++++--
>>  hw/xfree86/man/xorg.conf.man   |  6 +++++-
>>  include/xorg-config.h.in       |  6 ++++++
>>  5 files changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index d278c6c..b16d30a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -2043,6 +2043,8 @@ if test "x$XORG" = xyes; then
>>  	AC_SUBST(XF86CONFIGDIR)
>>  	CONFIGFILE="$sysconfdir/$XF86CONFIGFILE"
>>  	LOGPREFIX="Xorg."
>> +	XDG_DATA_HOME=".local/share"
>> +	XDG_DATA_HOME_LOGDIR="xorg"
>>  	AC_DEFINE(XORG_SERVER, 1, [Building Xorg server])
>>  	AC_DEFINE(XORGSERVER, 1, [Building Xorg server])
>>  	AC_DEFINE(XFree86Server, 1, [Building XFree86 server])
>> @@ -2057,6 +2059,8 @@ if test "x$XORG" = xyes; then
>>  	AC_DEFINE_DIR(DEFAULT_LIBRARY_PATH, libdir, [Default library install path])
>>  	AC_DEFINE_DIR(DEFAULT_LOGDIR, logdir, [Default log location])
>>  	AC_DEFINE_DIR(DEFAULT_LOGPREFIX, LOGPREFIX, [Default logfile prefix])
>> +	AC_DEFINE_DIR(DEFAULT_XDG_DATA_HOME, XDG_DATA_HOME, [Default XDG_DATA dir under HOME])
>> +	AC_DEFINE_DIR(DEFAULT_XDG_DATA_HOME_LOGDIR, XDG_DATA_HOME_LOGDIR, [Default log dir under XDG_DATA_HOME])
>>  	AC_DEFINE_UNQUOTED(__VENDORDWEBSUPPORT__, ["$VENDOR_WEB"], [Vendor web address for support])
>>  	if test "x$VGAHW" = xyes; then
>>  		AC_DEFINE(WITH_VGAHW, 1, [Building vgahw module])
>> diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
>> index 12a8771..5131db6 100644
>> --- a/hw/xfree86/common/xf86Helper.c
>> +++ b/hw/xfree86/common/xf86Helper.c
>> @@ -1220,13 +1220,29 @@ xf86ErrorF(const char *format, ...)
>>  void
>>  xf86LogInit(void)
>>  {
>> -    char *lf = NULL;
>> +    char *env, *lf = NULL;
>> +    char buf[PATH_MAX];
>>  
>>  #define LOGSUFFIX ".log"
>>  #define LOGOLDSUFFIX ".old"
>>  
>>      /* Get the log file name */
>>      if (xf86LogFileFrom == X_DEFAULT) {
>> +        /* When not running as root, we won't be able to write to /var/log */
>> +        if (geteuid() != 0) {
>> +            if ((env = getenv("XDG_DATA_HOME")))
>> +                snprintf(buf, sizeof(buf), "%s/%s", env,
>> +                         DEFAULT_XDG_DATA_HOME_LOGDIR);
>> +            else if ((env = getenv("HOME")))
>> +                snprintf(buf, sizeof(buf), "%s/%s/%s", env,
>> +                         DEFAULT_XDG_DATA_HOME, DEFAULT_XDG_DATA_HOME_LOGDIR);
>> +
>> +            if (env) {
>> +                (void)mkdir(buf, 0777);
> 
> I've merged the first patch, but I have two questions for this one:
> we can't really guarantee that XDG_DATA_HOME exists, especially when it's
> overridden for e.g. a test setup. I think we should do mkdir -p here
> to make sure we have the directory we want to write to. 

Ah yes, you're right I'll do a v3 fixing this.

> the second point: 755 would be enough? any specific reason for 777?

Using 0777 here is the normal way to invoke mkdir, the idea is that
you don't know whether the user wants 0755 or 0775 depending on how groups
are used on the system. So you just use 0777 and let the umask deal with it,
and depending on the umask you end up with either 0755 or 0775.

> PS: and of course this now brings up the question of whether we want
> $XDG_CONFIG_DIR for xorg.conf.d snippets...

Hmm, maybe but only when started without elevated rights... So probably
best to just not support this at all :)

Regards,

Hans