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

On a Thursday in 2021, Praveen K Paladugu wrote: > Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; 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