Skip to content

Commit

Permalink
Fix checking C code formatting
Browse files Browse the repository at this point in the history
Apparently, scripts/validate-c is not working in CI (or maybe
maintainers ignored the failures from it) -- current C code
gets some changes if we run indent on it.

This commit fixes this, simplifying things along the way.

 In particular:

1. Remove "validate" make target, add "cfmt" target that just runs
   indent on all *.c files in the repository (NOTE that *.h files
   are not included, as before).

   This may help a contributor to fix their code -- they just need
   to run "make cfmt" now instead of running "make validate" and
   copy-pasting the indent command and options from the hint.

2. Split GHA validate/misc into validate/release and validate/cfmt.
   The latter checks that the sources are not changed after "make cfmt".

3. Adds a few more options to indent. This was mostly motivated by
   trying to save the existing formatting, minimizing the amount of
   changes indent produces.

   The new options are:

   * -il0: sets the offset for goto labels to 0 (currently all labels
     but one are not indented -- let's keep it that way);

   * -ppi2: sets the indentation for nested preprocessor directives
     to 2 spaces (same as it is done in "SYS_memfd_create" defines);

   * -cp1: sets the indentation between #else / #endif and the
     following comment to 1 space.

4. Reformat the code using the new indent options.

5. Remove the now-unused script/{.validate,validate-c}.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Mar 17, 2021
1 parent b67deb5 commit 0045532
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 130 deletions.
23 changes: 19 additions & 4 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ jobs:
run: make cross


misc:
cfmt:
runs-on: ubuntu-20.04
steps:
- name: checkout
Expand All @@ -136,8 +136,23 @@ jobs:
- name: install deps
run: |
sudo apt -qq update
sudo apt -qq install libseccomp-dev indent
- name: make validate
run: make validate
sudo apt -qq install indent
- name: cfmt
run: |
make cfmt
git diff --exit-code
release:
runs-on: ubuntu-20.04
steps:
- name: checkout
uses: actions/checkout@v2
with:
fetch-depth: 0
- name: install deps
run: |
sudo apt -qq update
sudo apt -qq install libseccomp-dev
- name: make release
run: make release
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ clean:
rm -rf release
rm -rf man/man8

validate:
script/validate-c
cfmt: C_SRC=$(shell git ls-files '*.c' | grep -v '^vendor/')
cfmt:
indent -linux -l120 -il0 -ppi2 -cp1 -T size_t -T jmp_buf $(C_SRC)

shellcheck:
shellcheck tests/integration/*.bats tests/integration/*.sh tests/*.sh
Expand Down Expand Up @@ -150,5 +151,5 @@ localcross:
.PHONY: runc all recvtty static release dbuild lint man runcimage \
test localtest unittest localunittest integration localintegration \
rootlessintegration localrootlessintegration shell install install-bash \
install-man clean validate shfmt shellcheck \
install-man clean cfmt shfmt shellcheck \
vendor verify-dependencies cross localcross
67 changes: 33 additions & 34 deletions libcontainer/nsenter/cloned_binary.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,20 @@
# else
/* These values come from <https://fedora.juszkiewicz.com.pl/syscalls.html>. */
# warning "libc is outdated -- using hard-coded SYS_memfd_create"
# if defined(__x86_64__) // x86_64
# if defined(__x86_64__) // x86_64
# define SYS_memfd_create 319
# elif defined(__i386__) // i386
# elif defined(__i386__) // i386
# define SYS_memfd_create 356
# elif defined(__ia64__) // ia64
# elif defined(__ia64__) // ia64
# define SYS_memfd_create 1340
# elif defined(__arm__) // arm
# elif defined(__arm__) // arm
# define SYS_memfd_create 385
# elif defined(__aarch64__) // arm64
# elif defined(__aarch64__) // arm64
# define SYS_memfd_create 279
# elif defined(__ppc__) || defined(__ppc64__) // ppc + ppc64
# elif defined(__ppc__) || defined(__ppc64__)// ppc + ppc64
# define SYS_memfd_create 360
# elif defined(__s390__) || defined(__s390x__) // s390(x)
# elif defined(__s390__) || defined(__s390x__)
// s390(x)
# define SYS_memfd_create 350
# else
# error "unknown architecture -- cannot hard-code SYS_memfd_create"
Expand All @@ -101,7 +102,6 @@ int memfd_create(const char *name, unsigned int flags)
#endif
}


/* This comes directly from <linux/fcntl.h>. */
#ifndef F_LINUX_SPECIFIC_BASE
# define F_LINUX_SPECIFIC_BASE 1024
Expand All @@ -127,7 +127,7 @@ static void *must_realloc(void *ptr, size_t size)
void *old = ptr;
do {
ptr = realloc(old, size);
} while(!ptr);
} while (!ptr);
return ptr;
}

Expand All @@ -139,10 +139,10 @@ static void *must_realloc(void *ptr, size_t size)
static int is_self_cloned(void)
{
int fd, ret, is_cloned = 0;
struct stat statbuf = {};
struct statfs fsbuf = {};
struct stat statbuf = { };
struct statfs fsbuf = { };

fd = open("/proc/self/exe", O_RDONLY|O_CLOEXEC);
fd = open("/proc/self/exe", O_RDONLY | O_CLOEXEC);
if (fd < 0) {
fprintf(stderr, "you have no read access to runc binary file\n");
return -ENOTRECOVERABLE;
Expand Down Expand Up @@ -298,7 +298,7 @@ enum {
static int make_execfd(int *fdtype)
{
int fd = -1;
char template[PATH_MAX] = {0};
char template[PATH_MAX] = { 0 };
char *prefix = getenv("_LIBCONTAINER_STATEDIR");

if (!prefix || *prefix != '/')
Expand Down Expand Up @@ -327,7 +327,7 @@ static int make_execfd(int *fdtype)
*fdtype = EFD_FILE;
fd = open(prefix, O_TMPFILE | O_EXCL | O_RDWR | O_CLOEXEC, 0700);
if (fd >= 0) {
struct stat statbuf = {};
struct stat statbuf = { };
bool working_otmpfile = false;

/*
Expand Down Expand Up @@ -372,35 +372,35 @@ static int seal_execfd(int *fd, int fdtype)
switch (fdtype) {
case EFD_MEMFD:
return fcntl(*fd, F_ADD_SEALS, RUNC_MEMFD_SEALS);
case EFD_FILE: {
/* Need to re-open our pseudo-memfd as an O_PATH to avoid execve(2) giving -ETXTBSY. */
int newfd;
char fdpath[PATH_MAX] = {0};
case EFD_FILE:{
/* Need to re-open our pseudo-memfd as an O_PATH to avoid execve(2) giving -ETXTBSY. */
int newfd;
char fdpath[PATH_MAX] = { 0 };

if (fchmod(*fd, 0100) < 0)
return -1;
if (fchmod(*fd, 0100) < 0)
return -1;

if (snprintf(fdpath, sizeof(fdpath), "/proc/self/fd/%d", *fd) < 0)
return -1;
if (snprintf(fdpath, sizeof(fdpath), "/proc/self/fd/%d", *fd) < 0)
return -1;

newfd = open(fdpath, O_PATH | O_CLOEXEC);
if (newfd < 0)
return -1;
newfd = open(fdpath, O_PATH | O_CLOEXEC);
if (newfd < 0)
return -1;

close(*fd);
*fd = newfd;
return 0;
}
close(*fd);
*fd = newfd;
return 0;
}
default:
break;
break;
}
return -1;
}

static int try_bindfd(void)
{
int fd, ret = -1;
char template[PATH_MAX] = {0};
char template[PATH_MAX] = { 0 };
char *prefix = getenv("_LIBCONTAINER_STATEDIR");

if (!prefix || *prefix != '/')
Expand Down Expand Up @@ -428,7 +428,6 @@ static int try_bindfd(void)
if (mount("", template, "", MS_REMOUNT | MS_BIND | MS_RDONLY, "") < 0)
goto out_umount;


/* Get read-only handle that we're sure can't be made read-write. */
ret = open(template, O_PATH | O_CLOEXEC);

Expand Down Expand Up @@ -472,7 +471,7 @@ static ssize_t fd_to_fd(int outfd, int infd)
if (n < 0)
return -1;
nwritten += n;
} while(nwritten < nread);
} while (nwritten < nread);

total += nwritten;
}
Expand All @@ -483,7 +482,7 @@ static ssize_t fd_to_fd(int outfd, int infd)
static int clone_binary(void)
{
int binfd, execfd;
struct stat statbuf = {};
struct stat statbuf = { };
size_t sent = 0;
int fdtype = EFD_NONE;

Expand Down
28 changes: 14 additions & 14 deletions libcontainer/nsenter/nsexec.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct clone_t {
* Reserve some space for clone() to locate arguments
* and retcode in this place
*/
char stack[4096] __attribute__ ((aligned(16)));
char stack[4096] __attribute__((aligned(16)));
char stack_ptr[0];

/* There's two children. This is used to execute the different code. */
Expand Down Expand Up @@ -118,15 +118,15 @@ static int logfd = -1;
* it, namely (glibc 2.12).
*/
#if __GLIBC__ == 2 && __GLIBC_MINOR__ < 14
# define _GNU_SOURCE
# include "syscall.h"
# if !defined(SYS_setns) && defined(__NR_setns)
# define SYS_setns __NR_setns
# endif

#ifndef SYS_setns
# error "setns(2) syscall not supported by glibc version"
#endif
# define _GNU_SOURCE
# include "syscall.h"
# if !defined(SYS_setns) && defined(__NR_setns)
# define SYS_setns __NR_setns
# endif

# ifndef SYS_setns
# error "setns(2) syscall not supported by glibc version"
# endif

int setns(int fd, int nstype)
{
Expand All @@ -136,7 +136,7 @@ int setns(int fd, int nstype)

static void write_log_with_info(const char *level, const char *function, int line, const char *format, ...)
{
char message[1024] = {};
char message[1024] = { };

va_list args;

Expand Down Expand Up @@ -187,7 +187,7 @@ static int write_file(char *data, size_t data_len, char *pathfmt, ...)
goto out;
}

out:
out:
close(fd);
return ret;
}
Expand Down Expand Up @@ -328,14 +328,14 @@ static void update_oom_score_adj(char *data, size_t len)
}

/* A dummy function that just jumps to the given jumpval. */
static int child_func(void *arg) __attribute__ ((noinline));
static int child_func(void *arg) __attribute__((noinline));
static int child_func(void *arg)
{
struct clone_t *ca = (struct clone_t *)arg;
longjmp(*ca->env, ca->jmpval);
}

static int clone_parent(jmp_buf *env, int jmpval) __attribute__ ((noinline));
static int clone_parent(jmp_buf *env, int jmpval) __attribute__((noinline));
static int clone_parent(jmp_buf *env, int jmpval)
{
struct clone_t ca = {
Expand Down
33 changes: 0 additions & 33 deletions script/.validate

This file was deleted.

42 changes: 0 additions & 42 deletions script/validate-c

This file was deleted.

0 comments on commit 0045532

Please sign in to comment.