On 12/3/2021 11:25 AM, Ján Tomko wrote:
On a Thursday in 2021, Praveen K Paladugu wrote:
Signed-off-by: Praveen K Paladugu <pra...@linux.microsoft.com>

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

It is hard to follow all the formatting guidelines manually. So I primarily relied on running "indent" to handle all formatting as suggested at https://libvirt.org/coding-style.html#code-formatting-especially-for-new-code.

Is there a different tool I can manage all formatting with?


---
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

--
Regards,
Praveen K Paladugu


Reply via email to