bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> So this simplifies the code considerably, and in fact removes the
> whole retry thing completely.

We could do that, yes.  It looks good to me.

I had a different idea in mind though, which lets us keep the diagnostic
message, but only if given a --verbose flag.  We could extend --verbose
to include more diagnostic output in the future.  Please see the
attached patch.

I'm happy with either your patch or mine, depending on what people
prefer.

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 88800b9b2e..cd3069d7a8 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -66,7 +66,9 @@ Copyright (C) 1986-2022 Free Software Foundation, Inc.

 #endif /* !WINDOWSNT */

-#define DEFAULT_TIMEOUT (30)
+/* Seconds to wait before warning that the server hasn't responded.
+   Only applicable with --verbose and without any --timeout flag.  */
+#define DEFAULT_WARN_AFTER (30)

 #include <ctype.h>
 #include <errno.h>
@@ -110,6 +112,9 @@ #define DEFAULT_TIMEOUT (30)
 /* True means don't print messages for successful operations.  --quiet.  */
 static bool quiet;

+/* True means print verbose messages.  --verbose.  */
+static bool verbose;
+
 /* True means don't print values returned from emacs. --suppress-output.  */
 static bool suppress_output;

@@ -166,6 +171,7 @@ #define DEFAULT_TIMEOUT (30)
 static struct option const longopts[] =
 {
   { "no-wait", no_argument,       NULL, 'n' },
+  { "verbose", no_argument,       NULL, 'v' },
   { "quiet",   no_argument,       NULL, 'q' },
   { "suppress-output", no_argument, NULL, 'u' },
   { "eval",    no_argument,       NULL, 'e' },
@@ -191,7 +197,7 @@ #define DEFAULT_TIMEOUT (30)
 /* Short options, in the same order as the corresponding long options.
    There is no '-p' short option.  */
 static char const shortopts[] =
-  "nqueHVtca:F:w:"
+  "nqueHVtcva:F:w:"
 #ifdef SOCKETS_IN_FILE_SYSTEM
   "s:"
 #endif
@@ -488,6 +494,23 @@ message (bool is_error, const char *format, ...)
   va_end (args);
 }

+/* Print message only if we got the --verbose flag.  */
+static void print_verbose (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2);
+static void
+print_verbose (const char *format, ...)
+{
+  va_list args;
+  va_start (args, format);
+
+  if (verbose)
+    {
+      vfprintf (stderr, format, args);
+      fflush (stderr);
+    }
+
+  va_end (args);
+}
+
 /* Decode the options from argv and argc.
    The global variable 'optind' will say how many arguments we used up.  */

@@ -556,6 +579,10 @@ decode_options (int argc, char **argv)
          quiet = true;
          break;

+       case 'v':
+         verbose = true;
+         break;
+
        case 'u':
          suppress_output = true;
          break;
@@ -2144,40 +2171,49 @@ main (int argc, char **argv)
     }
   fflush (stdout);

-  set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
-  bool saw_response = false;
+  bool should_timeout = timeout > 0 || verbose;
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
     {
-      bool retry = true;
-      bool msg_showed = quiet;
+      if (should_timeout)
+       set_socket_timeout (emacs_socket,
+                           timeout > 0 ? timeout : DEFAULT_WARN_AFTER);
       do
        {
+       retry_recv:
          act_on_signals (emacs_socket);
          rl = recv (emacs_socket, string, BUFSIZ, 0);
-         retry = check_socket_timeout (rl);
-         if (retry && !saw_response)
+
+         /* Handle --timeout and --verbose.  */
+         if (should_timeout
+             /* Just retry if we got EINTR; it's not a timeout.  */
+             && errno != EINTR)
            {
-             if (timeout > 0)
-               {
-                 /* Don't retry if we were given a --timeout flag.  */
-                 fprintf (stderr, "\nServer not responding; timed out after 
%lu seconds",
-                          timeout);
-                 retry = false;
-               }
-             else if (!msg_showed)
+             should_timeout = false;
+             set_socket_timeout (emacs_socket, 0);
+
+             if (check_socket_timeout (rl))
                {
-                 msg_showed = true;
-                 fprintf (stderr, "\nServer not responding; use Ctrl+C to 
break");
+                 if (timeout > 0)
+                   {
+                     fprintf (stderr,
+                              "\nServer not responding; timed out after %lu 
seconds",
+                              timeout);
+                     break;
+                   }
+                 else /* if (verbose) */
+                   {
+                     print_verbose ("\nServer not responding; use Ctrl+C to 
break");
+                     goto retry_recv;
+                   }
                }
            }
        }
-      while ((rl < 0 && errno == EINTR) || retry);
+      while (rl < 0 && errno == EINTR);

       if (rl <= 0)
         break;

-      saw_response = true;
       string[rl] = '\0';

       /* Loop over all NL-terminated messages.  */