Re: [PATCH v4 08/15] daemon: use run-command api for async serving

Erik Faye-Lund [off-list ref] writes:

This is not particularly a good style.  Please make it more clear that we
freopen in inetd mode by writing it like this:

	if (inetd_mode) {
        	if (!freopen(...))
                	die_errno(...)
	}

On Thu, Oct 14, 2010 at 12:47 AM, Junio C Hamano [off-list ref] wrote:

Erik Faye-Lund [off-list ref] writes:

quoted

@@ -1017,7 +1005,12 @@ int main(int argc, char **argv)

                              continue;
                      }
              }
+             if (!strcmp(arg, "--serve")) {
+                     serve_mode = 1;
+                     continue;
+             }
              if (!strcmp(arg, "--inetd")) {
+                     serve_mode = 1;
                      inetd_mode = 1;
                      log_syslog = 1;
                      continue;

@@ -1161,12 +1154,12 @@ int main(int argc, char **argv)

              die("base-path '%s' does not exist or is not a directory",
                  base_path);

-     if (inetd_mode) {
+     if (serve_mode) {
              struct sockaddr_storage ss;
              struct sockaddr *peer = (struct sockaddr *)&ss;
              socklen_t slen = sizeof(ss);

-             if (!freopen("/dev/null", "w", stderr))
+             if (inetd_mode && !freopen("/dev/null", "w", stderr))
                      die_errno("failed to redirect stderr to /dev/null");

This is not particularly a good style.  Please make it more clear that we
freopen in inetd mode by writing it like this:

       if (inetd_mode) {
               if (!freopen(...))
                       die_errno(...)
       }

Much nicer, yeah. Now I'm tempted to do this also:

---8<---

diff --git a/daemon.c b/daemon.c
index 7f5d72f..11a5e06 100644
--- a/daemon.c
+++ b/daemon.c

@@ -1010,7 +1010,6 @@ int main(int argc, char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "--inetd")) {
-			serve_mode = 1;
 			inetd_mode = 1;
 			log_syslog = 1;
 			continue;

@@ -1159,7 +1158,7 @@ int main(int argc, char **argv)
 			die_errno("failed to redirect stderr to /dev/null");
 	}

-	if (serve_mode) {
+	if (inetd_mode || serve_mode) {
 		struct sockaddr_storage ss;
 		struct sockaddr *peer = (struct sockaddr *)&ss;
 		socklen_t slen = sizeof(ss);
---8<---

Erik Faye-Lund [off-list ref] writes:

On Thu, Oct 14, 2010 at 12:47 AM, Junio C Hamano [off-list ref] wrote:

quoted

Erik Faye-Lund [off-list ref] writes:
...

quoted

@@ -1017,7 +1005,12 @@ int main(int argc, char **argv)
+             if (inetd_mode && !freopen("/dev/null", "w", stderr))
                      die_errno("failed to redirect stderr to /dev/null");

This is not particularly a good style.  Please make it more clear that we
freopen in inetd mode by writing it like this:

       if (inetd_mode) {
               if (!freopen(...))
                       die_errno(...)
       }

Much nicer, yeah. Now I'm tempted to do this also:
...

Yeah, that is much much saner.  Thanks.