Re: [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<minari...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel