[PATCH] compositor: Verify XDG_RUNTIME_DIR variable

[Ander Conselvan de Oliveira]

Hi,

On 06/20/2012 09:31 AM, Pekka Paalanen wrote:
> Hi Martin
>
> On Mon, 18 Jun 2012 20:15:18 +0200
> Martin Minarik<minarik11 at student.fiit.stuba.sk>  wrote:
>
>> Print an user friendly error mesage when
>> the variable is not a valid directory.
>> ---
>>   src/compositor.c |   34 +++++++++++++++++++++++++++++-----
>>   1 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index d40a878..f5940aa 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -39,6 +39,7 @@
>>   #include<sys/wait.h>
>>   #include<sys/socket.h>
>>   #include<sys/utsname.h>
>> +#include<sys/stat.h>
>>   #include<unistd.h>
>>   #include<math.h>
>>   #include<linux/input.h>
>> @@ -3145,11 +3146,37 @@ load_module(const char *name, const char *entrypoint, void **handle)
>>   }
>>
>>   static const char xdg_error_message[] =
>> -	"fatal: environment variable XDG_RUNTIME_DIR is not set.\n"
>> +	"fatal: environment variable XDG_RUNTIME_DIR is not set.\n";
>> +
>> +static const char xdg_wrong_message[] =
>> +	"fatal: environment variable XDG_RUNTIME_DIR\n"
>> +	"is set to \"%s\". This is not a correct directory.\n";
>
> Let make that like this:
> "is set to \"%s\", which is not an existing directory.\n"

Wouldn't you also get this message if the env var points to a file? I'd 
go for "..., which is not a directory.\n"

> If you say "correct directory", the user starts to wonder
> what is *the* correct directory. Such information cannot be
> found in Google or anywhere else, because it depends, and
> there is no single correct answer.
>
> Also you only check that the directory exists, so let's
> say that.

There is a S_ISDIR check in the else case.

>> +
>> +static const char xdg_detail_message[] =
>>   	"Refer to your distribution on how to get it, or\n"
>>   	"http://www.freedesktop.org/wiki/Specifications/basedir-spec\n"
>>   	"on how to implement it.\n";
>>
>> +static void
>> +verify_xdg_runtime_dir()
>> +{
>> +	char * dir = getenv("XDG_RUNTIME_DIR");
>> +	struct stat s;
>> +
>> +	if (!dir) {
>> +		weston_log(xdg_error_message);
>> +		weston_log_continue(xdg_detail_message);
>> +		exit(EXIT_FAILURE);
>> +	} else {
>> +		stat(dir,&s);

You should also check stat()'s return value for errors.

Ander

>> +		if (!S_ISDIR(s.st_mode)) {
>> +			weston_log(xdg_wrong_message, dir);
>> +			weston_log_continue(xdg_detail_message);
>> +			exit(EXIT_FAILURE);
>> +		}
>> +	}
>> +}
>> +
>>   int main(int argc, char *argv[])
>>   {
>>   	int ret = EXIT_SUCCESS;
>> @@ -3196,10 +3223,7 @@ int main(int argc, char *argv[])
>>
>>   	weston_log_file_open(log);
>>   	
>> -	if (!getenv("XDG_RUNTIME_DIR")) {
>> -		weston_log(xdg_error_message);
>> -		exit(EXIT_FAILURE);
>> -	}
>> +	verify_xdg_runtime_dir();
>>
>>   	weston_log("%s\n"
>>   		   STAMP_SPACE "%s\n"
>
>
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel