[libvirt PATCH v2 01/17] util: fix indentation in virprocess.c

Ján Tomko jtomko at redhat.com
Fri Dec 3 17:25:28 UTC 2021


On a Thursday in 2021, Praveen K Paladugu wrote:
>Signed-off-by: Praveen K Paladugu <prapal at linux.microsoft.com>

Most of these indentation fixes here are against our coding style:
https://libvirt.org/coding-style.html

>---
> src/util/virprocess.c | 501 +++++++++++++++++++++---------------------
> 1 file changed, 249 insertions(+), 252 deletions(-)
>
>diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>index 7b0ad9c97b..8288e71f67 100644
>--- a/src/util/virprocess.c
>+++ b/src/util/virprocess.c
>@@ -25,36 +25,36 @@
> #include <fcntl.h>
> #include <signal.h>
> #ifndef WIN32
>-# include <sys/wait.h>
>+#include <sys/wait.h>
> #endif
> #include <unistd.h>

This is intentional spacing to make it obvious which directives
are conditional. It's enforced by cppi(1) which is run as a part
of our tests, if installed.

>-static inline int setns(int fd, int nstype)
>+static inline int
>+setns(int fd, int nstype)
> {

This is good and the style we try to use for new code.

>@@ -115,15 +118,11 @@ static inline int setns(int fd G_GNUC_UNUSED, int nstype G_GNUC_UNUSED)
>
> VIR_ENUM_IMPL(virProcessSchedPolicy,
>               VIR_PROC_POLICY_LAST,
>-              "none",
>-              "batch",
>-              "idle",
>-              "fifo",
>-              "rr",
>-);
>+              "none", "batch", "idle", "fifo", "rr",);

One line per enum value makes for nicer diffs

>
>
> #ifndef WIN32
>+
> /**
>  * virProcessTranslateStatus:
>  * @status: child exit status to translate
>@@ -136,12 +135,11 @@ char *
> virProcessTranslateStatus(int status)
> {
>     char *buf;
>+
>     if (WIFEXITED(status)) {
>-        buf = g_strdup_printf(_("exit status %d"),
>-                              WEXITSTATUS(status));
>+        buf = g_strdup_printf(_("exit status %d"), WEXITSTATUS(status));
>     } else if (WIFSIGNALED(status)) {
>-        buf = g_strdup_printf(_("fatal signal %d"),
>-                              WTERMSIG(status));
>+        buf = g_strdup_printf(_("fatal signal %d"), WTERMSIG(status));
>     } else {
>         buf = g_strdup_printf(_("invalid value %d"), status);
>     }
>@@ -175,8 +173,7 @@ virProcessAbort(pid_t pid)
>      */
>     saved_errno = errno;
>     VIR_DEBUG("aborting child process %d", pid);
>-    while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
>-           errno == EINTR);
>+    while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR);
>     if (ret == pid) {
>         tmp = virProcessTranslateStatus(status);
>         VIR_DEBUG("process has ended: %s", tmp);
>@@ -185,8 +182,7 @@ virProcessAbort(pid_t pid)
>         VIR_DEBUG("trying SIGTERM to child process %d", pid);
>         kill(pid, SIGTERM);
>         g_usleep(10 * 1000);
>-        while ((ret = waitpid(pid, &status, WNOHANG)) == -1 &&
>-               errno == EINTR);
>+        while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR);
>         if (ret == pid) {
>             tmp = virProcessTranslateStatus(status);
>             VIR_DEBUG("process has ended: %s", tmp);
>@@ -194,8 +190,7 @@ virProcessAbort(pid_t pid)
>         } else if (ret == 0) {
>             VIR_DEBUG("trying SIGKILL to child process %d", pid);
>             kill(pid, SIGKILL);
>-            while ((ret = waitpid(pid, &status, 0)) == -1 &&
>-                   errno == EINTR);
>+            while ((ret = waitpid(pid, &status, 0)) == -1 && errno == EINTR);
>             if (ret == pid) {
>                 tmp = virProcessTranslateStatus(status);
>                 VIR_DEBUG("process has ended: %s", tmp);
>@@ -246,8 +241,7 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw)
>     }
>
>     /* Wait for intermediate process to exit */
>-    while ((ret = waitpid(pid, &status, 0)) == -1 &&
>-           errno == EINTR);
>+    while ((ret = waitpid(pid, &status, 0)) == -1 && errno == EINTR);
>
>     if (ret == -1) {
>         virReportSystemError(errno, _("unable to wait for process %lld"),
>@@ -289,7 +283,7 @@ void
> virProcessAbort(pid_t pid)
> {
>     /* Not yet ported to mingw.  Any volunteers?  */
>-    VIR_DEBUG("failed to reap child %lld, abandoning it", (long long)pid);
>+    VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid);
> }
>
>
>@@ -305,7 +299,8 @@ virProcessWait(pid_t pid, int *exitstatus G_GNUC_UNUSED, bool raw G_GNUC_UNUSED)
>
>
> /* send signal to a single process */
>-int virProcessKill(pid_t pid, int sig)
>+int
>+virProcessKill(pid_t pid, int sig)
> {
>     if (pid <= 1) {
>         errno = ESRCH;
>@@ -315,44 +310,45 @@ int virProcessKill(pid_t pid, int sig)
> #ifdef WIN32
>     /* Mingw / Windows don't have many signals (AFAIK) */
>     switch (sig) {
>-    case SIGINT:

Aligning 'case's with the 'switch' keyword is intentional.

>-        /* This does a Ctrl+C equiv */
>-        if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, pid)) {
>-            errno = ESRCH;
>-            return -1;
>-        }
>-        break;
>@@ -432,21 +432,21 @@ virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay, boo
>         int rc;
>
>         if (i == 0) {
>-            signum = SIGTERM; /* kindly suggest it should exit */
>+            signum = SIGTERM;   /* kindly suggest it should exit */

Indenting the comment by three spaces seems an odd choice.

>         } else if (i == 50 && force) {
>             VIR_DEBUG("Timed out waiting after SIGTERM to process %lld, "
>-                      "sending SIGKILL", (long long)pid);
>+                      "sending SIGKILL", (long long) pid);
>             /* No SIGKILL kill on Win32 ! Use SIGABRT instead which our
>              * virProcessKill proc will handle more or less like SIGKILL */
> #ifdef WIN32
>-            signum = SIGABRT; /* kill it after a grace period */
>+            signum = SIGABRT;   /* kill it after a grace period */
>             signame = "ABRT";
> #else
>-            signum = SIGKILL; /* kill it after a grace period */
>+            signum = SIGKILL;   /* kill it after a grace period */
>             signame = "KILL";
> #endif
>         } else {
>-            signum = 0; /* Just check for existence */
>+            signum = 0;         /* Just check for existence */
>         }
>
>         if (group)
>@@ -457,8 +457,9 @@ virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay, boo
>         if (rc < 0) {
>             if (errno != ESRCH) {
>                 virReportSystemError(errno,
>-                                     _("Failed to terminate process %lld with SIG%s"),
>-                                     (long long)pid, signame);
>+                                     _
>+                                     ("Failed to terminate process %lld with SIG%s"),

The macro invocation should be on the same line as its parameter.

>+                                     (long long) pid, signame);

No need for the space after the cast.

>                 return -1;
>             }
>             return signum == SIGTERM ? 0 : 1;

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211203/e5cc45cf/attachment-0001.sig>


More information about the libvir-list mailing list