Merge branch 'jk/credential-cache-comment-exit' into maint
authorJunio C Hamano <gitster@pobox.com>
Fri, 15 Apr 2016 01:37:16 +0000 (18:37 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 15 Apr 2016 01:37:16 +0000 (18:37 -0700)
A code clarification.

* jk/credential-cache-comment-exit:
credential-cache--daemon: clarify "exit" action semantics

1  2 
credential-cache--daemon.c
index caef21e4fc91898f209709f723de1df5afc66a66,8a3d0b7e2c0d1592b8fa5445d7ef4e47844649b5..291c0fd5e935b5abedc629697d44e5a9f57727bd
@@@ -96,12 -96,12 +96,12 @@@ static int read_request(FILE *fh, struc
        static struct strbuf item = STRBUF_INIT;
        const char *p;
  
 -      strbuf_getline(&item, fh, '\n');
 +      strbuf_getline_lf(&item, fh);
        if (!skip_prefix(item.buf, "action=", &p))
                return error("client sent bogus action line: %s", item.buf);
        strbuf_addstr(action, p);
  
 -      strbuf_getline(&item, fh, '\n');
 +      strbuf_getline_lf(&item, fh);
        if (!skip_prefix(item.buf, "timeout=", &p))
                return error("client sent bogus timeout line: %s", item.buf);
        *timeout = atoi(p);
@@@ -126,8 -126,17 +126,17 @@@ static void serve_one_client(FILE *in, 
                        fprintf(out, "password=%s\n", e->item.password);
                }
        }
-       else if (!strcmp(action.buf, "exit"))
+       else if (!strcmp(action.buf, "exit")) {
+               /*
+                * It's important that we clean up our socket first, and then
+                * signal the client only once we have finished the cleanup.
+                * Calling exit() directly does this, because we clean up in
+                * our atexit() handler, and then signal the client when our
+                * process actually ends, which closes the socket and gives
+                * them EOF.
+                */
                exit(0);
+       }
        else if (!strcmp(action.buf, "erase"))
                remove_credential(&c);
        else if (!strcmp(action.buf, "store")) {
@@@ -215,7 -224,7 +224,7 @@@ static const char permissions_advice[] 
  "users may be able to read your cached credentials. Consider running:\n"
  "\n"
  "     chmod 0700 %s";
 -static void check_socket_directory(const char *path)
 +static void init_socket_directory(const char *path)
  {
        struct stat st;
        char *path_copy = xstrdup(path);
        if (!stat(dir, &st)) {
                if (st.st_mode & 077)
                        die(permissions_advice, dir);
 -              free(path_copy);
 -              return;
 +      } else {
 +              /*
 +               * We must be sure to create the directory with the correct mode,
 +               * not just chmod it after the fact; otherwise, there is a race
 +               * condition in which somebody can chdir to it, sleep, then try to open
 +               * our protected socket.
 +               */
 +              if (safe_create_leading_directories_const(dir) < 0)
 +                      die_errno("unable to create directories for '%s'", dir);
 +              if (mkdir(dir, 0700) < 0)
 +                      die_errno("unable to mkdir '%s'", dir);
        }
  
 -      /*
 -       * We must be sure to create the directory with the correct mode,
 -       * not just chmod it after the fact; otherwise, there is a race
 -       * condition in which somebody can chdir to it, sleep, then try to open
 -       * our protected socket.
 -       */
 -      if (safe_create_leading_directories_const(dir) < 0)
 -              die_errno("unable to create directories for '%s'", dir);
 -      if (mkdir(dir, 0700) < 0)
 -              die_errno("unable to mkdir '%s'", dir);
 +      if (chdir(dir))
 +              /*
 +               * We don't actually care what our cwd is; we chdir here just to
 +               * be a friendly daemon and avoid tying up our original cwd.
 +               * If this fails, it's OK to just continue without that benefit.
 +               */
 +              ;
 +
        free(path_copy);
  }
  
@@@ -271,10 -273,7 +280,10 @@@ int main(int argc, const char **argv
        if (!socket_path)
                usage_with_options(usage, options);
  
 -      check_socket_directory(socket_path);
 +      if (!is_absolute_path(socket_path))
 +              die("socket directory must be an absolute path");
 +
 +      init_socket_directory(socket_path);
        register_tempfile(&socket_file, socket_path);
  
        if (ignore_sighup)