Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/simplify scripts/validate-c, fix *.c formatting #2860

Merged
merged 4 commits into from
Mar 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
66 changes: 32 additions & 34 deletions libcontainer/nsenter/cloned_binary.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,19 @@
# else
/* These values come from <https://fedora.juszkiewicz.com.pl/syscalls.html>. */
# warning "libc is outdated -- using hard-coded SYS_memfd_create"
cyphar marked this conversation as resolved.
Show resolved Hide resolved
# if defined(__x86_64__) // x86_64
# if defined(__x86_64__)
# define SYS_memfd_create 319
# elif defined(__i386__) // i386
# elif defined(__i386__)
# define SYS_memfd_create 356
# elif defined(__ia64__) // ia64
# elif defined(__ia64__)
# define SYS_memfd_create 1340
# elif defined(__arm__) // arm
# elif defined(__arm__)
# define SYS_memfd_create 385
# elif defined(__aarch64__) // arm64
# elif defined(__aarch64__)
# define SYS_memfd_create 279
# elif defined(__ppc__) || defined(__ppc64__) // ppc + ppc64
# elif defined(__ppc__) || defined(__ppc64__)
# define SYS_memfd_create 360
# elif defined(__s390__) || defined(__s390x__) // s390(x)
# elif defined(__s390__) || defined(__s390x__)
# define SYS_memfd_create 350
# else
# error "unknown architecture -- cannot hard-code SYS_memfd_create"
Expand All @@ -101,7 +101,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 +126,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 +138,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 +297,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 +326,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 +371,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: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the diff, this one surprised me (that it's removing the whitespace here,. but adding whitespace everywhere else)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is removing the whitespace here as I have removed the { } block (see description in the first commit).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I was talking about a different hunk.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess indent has a flag for this one, too, but I'm too lazy to look for one, since this change only happens once, so adding that flag won't result in drastic changes in the patch size, and I don't want to have too many flags.

/* 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. */
kolyshkin marked this conversation as resolved.
Show resolved Hide resolved
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 +427,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 +470,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 +481,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
94 changes: 47 additions & 47 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 @@ -102,31 +102,31 @@ static int logfd = -1;
* List of netlink message types sent to us as part of bootstrapping the init.
* These constants are defined in libcontainer/message_linux.go.
*/
#define INIT_MSG 62000
#define INIT_MSG 62000
#define CLONE_FLAGS_ATTR 27281
#define NS_PATHS_ATTR 27282
#define UIDMAP_ATTR 27283
#define GIDMAP_ATTR 27284
#define UIDMAP_ATTR 27283
#define GIDMAP_ATTR 27284
#define SETGROUP_ATTR 27285
#define OOM_SCORE_ADJ_ATTR 27286
#define ROOTLESS_EUID_ATTR 27287
#define UIDMAPPATH_ATTR 27288
#define GIDMAPPATH_ATTR 27289
#define UIDMAPPATH_ATTR 27288
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the alignment still off for the last two?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a mix of tabs and spaces, now it's just tabs and it looks aligned for tabstep=8.

GH does not use tabstep=8 so probably why it looks skewed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you can adjust with ?ts=<width> when viewing; https://github.com/tiimgreen/github-cheat-sheet#adjust-tab-space

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah does not seem to work when viewing diffs :-\

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😕😕😞😞😩

#define GIDMAPPATH_ATTR 27289

/*
* Use the raw syscall for versions of glibc which don't include a function for
* 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 Expand Up @@ -749,34 +749,34 @@ void nsexec(void)
bail("failed to sync with child: write(SYNC_USERMAP_ACK)");
}
break;
case SYNC_RECVPID_PLS:{
first_child = child;

/* Get the init_func pid. */
if (read(syncfd, &child, sizeof(child)) != sizeof(child)) {
kill(first_child, SIGKILL);
bail("failed to sync with child: read(childpid)");
}

/* Send ACK. */
s = SYNC_RECVPID_ACK;
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
kill(first_child, SIGKILL);
kill(child, SIGKILL);
bail("failed to sync with child: write(SYNC_RECVPID_ACK)");
}

/* Send the init_func pid back to our parent.
*
* Send the init_func pid and the pid of the first child back to our parent.
* We need to send both back because we can't reap the first child we created (CLONE_PARENT).
* It becomes the responsibility of our parent to reap the first child.
*/
len = dprintf(pipenum, "{\"pid\": %d, \"pid_first\": %d}\n", child, first_child);
if (len < 0) {
kill(child, SIGKILL);
bail("unable to generate JSON for child pid");
}
case SYNC_RECVPID_PLS:
first_child = child;

/* Get the init_func pid. */
if (read(syncfd, &child, sizeof(child)) != sizeof(child)) {
kill(first_child, SIGKILL);
bail("failed to sync with child: read(childpid)");
}

/* Send ACK. */
s = SYNC_RECVPID_ACK;
if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
kill(first_child, SIGKILL);
kill(child, SIGKILL);
bail("failed to sync with child: write(SYNC_RECVPID_ACK)");
}

/* Send the init_func pid back to our parent.
*
* Send the init_func pid and the pid of the first child back to our parent.
* We need to send both back because we can't reap the first child we created (CLONE_PARENT).
* It becomes the responsibility of our parent to reap the first child.
*/
len = dprintf(pipenum, "{\"pid\": %d, \"pid_first\": %d}\n",
child, first_child);
if (len < 0) {
kill(child, SIGKILL);
bail("unable to generate JSON for child pid");
}
break;
case SYNC_CHILD_READY:
Expand Down
33 changes: 0 additions & 33 deletions script/.validate

This file was deleted.

Loading