[libvirt PATCH v2 01/17] util: fix indentation in virprocess.c
Praveen K Paladugu
prapal at linux.microsoft.com
Fri Dec 3 17:33:26 UTC 2021
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 <prapal at 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
More information about the libvir-list
mailing list