The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2723
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Hello,
Some redundancy functions in pam_cgfs & cgfs are removed.
And dependencies of pam_cgfs from cap & log are removed.
Thanks.
From a6de11a79b60a1be47df83cfaaf98b0d3f2734c5 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.je...@samsung.com>
Date: Fri, 9 Nov 2018 13:43:41 +0900
Subject: [PATCH 1/5] pam_cgfs: remove redundancy file utils
Signed-off-by: 2xsec <dh48.je...@samsung.com>
---
src/lxc/pam/pam_cgfs.c | 104 ++++++-----------------------------------
1 file changed, 15 insertions(+), 89 deletions(-)
diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c
index 855a40f87..39e483c86 100644
--- a/src/lxc/pam/pam_cgfs.c
+++ b/src/lxc/pam/pam_cgfs.c
@@ -126,7 +126,6 @@ static void must_append_controller(char **klist, char
**nlist, char ***clist,
static void must_append_string(char ***list, char *entry);
static void mysyslog(int err, const char *format, ...)
__attribute__((sentinel));
static char *read_file(char *fnam);
-static int read_from_file(const char *filename, void* buf, size_t count);
static int recursive_rmdir(char *dirname);
static inline void set_bit(unsigned bit, uint32_t *bitarr)
{
@@ -136,9 +135,6 @@ static bool string_in_list(char **list, const char *entry);
static char *string_join(const char *sep, const char **parts, bool
use_as_prefix);
static void trim(char *s);
static bool write_int(char *path, int v);
-static ssize_t write_nointr(int fd, const void* buf, size_t count);
-static int write_to_file(const char *filename, const void *buf, size_t count,
- bool add_newline);
/* cgroupfs prototypes. */
static bool cg_belongs_to_uid_gid(const char *path, uid_t uid, gid_t gid);
@@ -1738,49 +1734,6 @@ static ssize_t cg_get_max_cpus(char *cpulist)
return cpus;
}
-static ssize_t write_nointr(int fd, const void* buf, size_t count)
-{
- ssize_t ret;
-
-again:
- ret = write(fd, buf, count);
- if (ret < 0 && errno == EINTR)
- goto again;
-
- return ret;
-}
-
-static int write_to_file(const char *filename, const void* buf, size_t count,
bool add_newline)
-{
- int fd, saved_errno;
- ssize_t ret;
-
- fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0666);
- if (fd < 0)
- return -1;
-
- ret = write_nointr(fd, buf, count);
- if (ret < 0)
- goto out_error;
- if ((size_t)ret != count)
- goto out_error;
-
- if (add_newline) {
- ret = write_nointr(fd, "\n", 1);
- if (ret != 1)
- goto out_error;
- }
-
- close(fd);
- return 0;
-
-out_error:
- saved_errno = errno;
- close(fd);
- errno = saved_errno;
- return -1;
-}
-
#define __ISOL_CPUS "/sys/devices/system/cpu/isolated"
static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
{
@@ -1905,7 +1858,7 @@ static bool cg_filter_and_set_cpus(char *path, bool
am_initialized)
free(fpath);
fpath = must_make_path(path, "cpuset.cpus", NULL);
- ret = write_to_file(fpath, cpulist, strlen(cpulist), false);
+ ret = lxc_write_to_file(fpath, cpulist, strlen(cpulist), false, 0660);
if (ret < 0) {
pam_cgfs_debug("Could not write cpu list to: %s\n", fpath);
goto on_error;
@@ -1929,37 +1882,6 @@ static bool cg_filter_and_set_cpus(char *path, bool
am_initialized)
return bret;
}
-int read_from_file(const char *filename, void* buf, size_t count)
-{
- int fd = -1, saved_errno;
- ssize_t ret;
-
- fd = open(filename, O_RDONLY | O_CLOEXEC);
- if (fd < 0)
- return -1;
-
- if (!buf || !count) {
- char buf2[100];
- size_t count2 = 0;
-
- while ((ret = read(fd, buf2, 100)) > 0)
- count2 += ret;
- if (ret >= 0)
- ret = count2;
- } else {
- memset(buf, 0, count);
- ret = read(fd, buf, count);
- }
-
- if (ret < 0)
- pam_cgfs_debug("read %s: %s", filename, strerror(errno));
-
- saved_errno = errno;
- close(fd);
- errno = saved_errno;
- return ret;
-}
-
/* Copy contents of parent(@path)/@file to @path/@file */
static bool cg_copy_parent_file(char *path, char *file)
{
@@ -1977,19 +1899,23 @@ static bool cg_copy_parent_file(char *path, char *file)
*lastslash = '\0';
fpath = must_make_path(path, file, NULL);
- len = read_from_file(fpath, NULL, 0);
- if (len <= 0)
+ len = lxc_read_from_file(fpath, NULL, 0);
+ if (len <= 0) {
+ pam_cgfs_debug("Failed to read %s: %s", fpath, strerror(errno));
goto bad;
+ }
value = must_alloc(len + 1);
- if (read_from_file(fpath, value, len) != len)
+ if (lxc_read_from_file(fpath, value, len) != len) {
+ pam_cgfs_debug("Failed to read %s: %s", fpath, strerror(errno));
goto bad;
+ }
free(fpath);
*lastslash = oldv;
fpath = must_make_path(path, file, NULL);
- ret = write_to_file(fpath, value, len, false);
+ ret = lxc_write_to_file(fpath, value, len, false, 0660);
if (ret < 0)
pam_cgfs_debug("Unable to write %s to %s", value, fpath);
@@ -2018,8 +1944,8 @@ static bool cgv1_handle_root_cpuset_hierarchy(struct
cgv1_hierarchy *h)
clonechildrenpath = must_make_path(h->mountpoint,
"cgroup.clone_children", NULL);
- if (read_from_file(clonechildrenpath, &v, 1) < 0) {
- pam_cgfs_debug("Failed to read '%s'", clonechildrenpath);
+ if (lxc_read_from_file(clonechildrenpath, &v, 1) < 0) {
+ pam_cgfs_debug("Failed to read %s: %s", clonechildrenpath,
strerror(errno));
free(clonechildrenpath);
return false;
}
@@ -2029,7 +1955,7 @@ static bool cgv1_handle_root_cpuset_hierarchy(struct
cgv1_hierarchy *h)
return true;
}
- if (write_to_file(clonechildrenpath, "1", 1, false) < 0) {
+ if (lxc_write_to_file(clonechildrenpath, "1", 1, false, 0660) < 0) {
/* Set clone_children so children inherit our settings */
pam_cgfs_debug("Failed to write 1 to %s", clonechildrenpath);
free(clonechildrenpath);
@@ -2077,8 +2003,8 @@ static bool cgv1_handle_cpuset_hierarchy(struct
cgv1_hierarchy *h,
return true;
}
- if (read_from_file(clonechildrenpath, &v, 1) < 0) {
- pam_cgfs_debug("Failed to read '%s'", clonechildrenpath);
+ if (lxc_read_from_file(clonechildrenpath, &v, 1) < 0) {
+ pam_cgfs_debug("Failed to read %s: %s", clonechildrenpath,
strerror(errno));
free(clonechildrenpath);
free(cgpath);
return false;
@@ -2108,7 +2034,7 @@ static bool cgv1_handle_cpuset_hierarchy(struct
cgv1_hierarchy *h,
}
free(cgpath);
- if (write_to_file(clonechildrenpath, "1", 1, false) < 0) {
+ if (lxc_write_to_file(clonechildrenpath, "1", 1, false, 0660) < 0) {
/* Set clone_children so children inherit our settings */
pam_cgfs_debug("Failed to write 1 to %s", clonechildrenpath);
free(clonechildrenpath);
From f25a2044bf08648a3c91d0b130069c8e96d4b099 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.je...@samsung.com>
Date: Fri, 9 Nov 2018 14:10:46 +0900
Subject: [PATCH 2/5] cgfs: remove redundancy utils
Signed-off-by: 2xsec <dh48.je...@samsung.com>
---
src/lxc/cgroups/cgfsng.c | 20 +++++++-------------
src/lxc/pam/pam_cgfs.c | 19 ++++++-------------
2 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 7474ba140..1631319ec 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -84,12 +84,6 @@ static void free_string_list(char **clist)
free(clist);
}
-/* Allocate a pointer, do not fail. */
-static void *must_alloc(size_t sz)
-{
- return must_realloc(NULL, sz);
-}
-
/* Given a pointer to a null-terminated array of pointers, realloc to add one
* entry, and point the new entry to NULL. Do not fail. Return the index to the
* second-to-last entry - that is, the one which is now available for use
@@ -134,7 +128,7 @@ static char *cg_legacy_must_prefix_named(char *entry)
char *prefixed;
len = strlen(entry);
- prefixed = must_alloc(len + 6);
+ prefixed = must_realloc(NULL, len + 6);
memcpy(prefixed, "name=", STRLITERALLEN("name="));
memcpy(prefixed + STRLITERALLEN("name="), entry, len);
@@ -541,7 +535,7 @@ static bool copy_parent_file(char *path, char *file)
if (len <= 0)
goto on_error;
- value = must_alloc(len + 1);
+ value = must_realloc(NULL, len + 1);
ret = lxc_read_from_file(fpath, value, len);
if (ret != len)
goto on_error;
@@ -824,7 +818,7 @@ static struct hierarchy *add_hierarchy(struct hierarchy
***h, char **clist, char
struct hierarchy *new;
int newentry;
- new = must_alloc(sizeof(*new));
+ new = must_realloc(NULL, sizeof(*new));
new->controllers = clist;
new->mountpoint = mountpoint;
new->container_base_path = container_base_path;
@@ -863,7 +857,7 @@ static char *cg_hybrid_get_mountpoint(char *line)
*p2 = '\0';
len = strlen(p);
- sret = must_alloc(len + 1);
+ sret = must_realloc(NULL, len + 1);
memcpy(sret, p, len);
sret[len] = '\0';
return sret;
@@ -879,7 +873,7 @@ static char *copy_to_eol(char *p)
return NULL;
len = p2 - p;
- sret = must_alloc(len + 1);
+ sret = must_realloc(NULL, len + 1);
memcpy(sret, p, len);
sret[len] = '\0';
return sret;
@@ -1466,7 +1460,7 @@ __cgfsng_ops static inline bool
cgfsng_payload_create(struct cgroup_ops *ops,
}
len = strlen(tmp) + 5; /* leave room for -NNN\0 */
- container_cgroup = must_alloc(len);
+ container_cgroup = must_realloc(NULL, len);
(void)strlcpy(container_cgroup, tmp, len);
free(tmp);
offset = container_cgroup + len - 5;
@@ -2110,7 +2104,7 @@ static int __cg_unified_attach(const struct hierarchy *h,
const char *name,
len = strlen(base_path) + STRLITERALLEN("/lxc-1000") +
STRLITERALLEN("/cgroup-procs");
- full_path = must_alloc(len + 1);
+ full_path = must_realloc(NULL, len + 1);
do {
if (idx)
ret = snprintf(full_path, len + 1, "%s/lxc-%d",
diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c
index 39e483c86..737935640 100644
--- a/src/lxc/pam/pam_cgfs.c
+++ b/src/lxc/pam/pam_cgfs.c
@@ -119,7 +119,6 @@ static inline bool is_set(unsigned bit, uint32_t *bitarr)
static bool is_lxcfs(const char *line);
static bool is_cgv1(char *line);
static bool is_cgv2(char *line);
-static void *must_alloc(size_t sz);
static void must_add_to_list(char ***clist, char *entry);
static void must_append_controller(char **klist, char **nlist, char ***clist,
char *entry);
@@ -388,12 +387,6 @@ static void trim(char *s)
s[--len] = '\0';
}
-/* Allocate pointer; do not fail. */
-static void *must_alloc(size_t sz)
-{
- return must_realloc(NULL, sz);
-}
-
/* Make allocated copy of string. End of string is taken to be '\n'. */
static char *copy_to_eol(char *s)
{
@@ -405,7 +398,7 @@ static char *copy_to_eol(char *s)
return NULL;
len = newline - s;
- sret = must_alloc(len + 1);
+ sret = must_realloc(NULL, len + 1);
memcpy(sret, s, len);
sret[len] = '\0';
@@ -603,7 +596,7 @@ static char *get_mountpoint(char *line)
*p2 = '\0';
len = strlen(p);
- sret = must_alloc(len + 1);
+ sret = must_realloc(NULL, len + 1);
memcpy(sret, p, len);
sret[len] = '\0';
@@ -775,7 +768,7 @@ static char *cgv1_must_prefix_named(char *entry)
size_t len;
len = strlen(entry);
- s = must_alloc(len + 6);
+ s = must_realloc(NULL, len + 6);
ret = snprintf(s, len + 6, "name=%s", entry);
if (ret < 0 || (size_t)ret >= (len + 6)) {
@@ -937,7 +930,7 @@ static void cgv1_add_controller(char **clist, char
*mountpoint, char *base_cgrou
struct cgv1_hierarchy *new;
int newentry;
- new = must_alloc(sizeof(*new));
+ new = must_realloc(NULL, sizeof(*new));
new->controllers = clist;
new->mountpoint = mountpoint;
@@ -964,7 +957,7 @@ static void cgv2_add_controller(char **clist, char
*mountpoint, char *base_cgrou
struct cgv2_hierarchy *new;
int newentry;
- new = must_alloc(sizeof(*new));
+ new = must_realloc(NULL, sizeof(*new));
new->controllers = clist;
new->mountpoint = mountpoint;
@@ -1905,7 +1898,7 @@ static bool cg_copy_parent_file(char *path, char *file)
goto bad;
}
- value = must_alloc(len + 1);
+ value = must_realloc(NULL, len + 1);
if (lxc_read_from_file(fpath, value, len) != len) {
pam_cgfs_debug("Failed to read %s: %s", fpath, strerror(errno));
goto bad;
From c4a090bebfb28a35975ee4317326e82bf2756707 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.je...@samsung.com>
Date: Fri, 9 Nov 2018 16:06:33 +0900
Subject: [PATCH 3/5] pam_cgfs: remove dependency from cap & log
Signed-off-by: 2xsec <dh48.je...@samsung.com>
---
src/lxc/Makefile.am | 2 --
src/lxc/file_utils.c | 3 ---
src/lxc/pam/pam_cgfs.c | 3 ++-
src/lxc/string_utils.c | 4 +---
4 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index c3714b8d8..9390055c1 100644
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -424,9 +424,7 @@ if HAVE_PAM
pam_LTLIBRARIES = pam_cgfs.la
pam_cgfs_la_SOURCES = pam/pam_cgfs.c \
- caps.c caps.h \
file_utils.c file_utils.h \
- log.c log.h \
macro.h \
string_utils.c string_utils.h
diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c
index 485a7843e..f89aa638d 100644
--- a/src/lxc/file_utils.c
+++ b/src/lxc/file_utils.c
@@ -30,12 +30,9 @@
#include "config.h"
#include "file_utils.h"
-#include "log.h"
#include "macro.h"
#include "string.h"
-lxc_log_define(file_utils, lxc);
-
int lxc_write_to_file(const char *filename, const void *buf, size_t count,
bool add_newline, mode_t mode)
{
diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c
index 737935640..4a45600ea 100644
--- a/src/lxc/pam/pam_cgfs.c
+++ b/src/lxc/pam/pam_cgfs.c
@@ -57,8 +57,9 @@
#include <unistd.h>
#include "config.h"
+#include "file_utils.h"
#include "macro.h"
-#include "utils.h"
+#include "string_utils.h"
#define PAM_SM_SESSION
#include <security/_pam_macros.h>
diff --git a/src/lxc/string_utils.c b/src/lxc/string_utils.c
index c4b915339..0d7538c1f 100644
--- a/src/lxc/string_utils.c
+++ b/src/lxc/string_utils.c
@@ -29,6 +29,7 @@
#include <inttypes.h>
#include <libgen.h>
#include <pthread.h>
+#include <stdarg.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
@@ -43,7 +44,6 @@
#include <unistd.h>
#include "config.h"
-#include "log.h"
#include "lxclock.h"
#include "macro.h"
#include "namespace.h"
@@ -58,8 +58,6 @@
#include "include/strlcat.h"
#endif
-lxc_log_define(string_utils, lxc);
-
char **lxc_va_arg_list_to_argv(va_list ap, size_t skip, int do_strdup)
{
va_list ap2;
From 7be6bcd523d06a27fa6e611dd822142e9aea6da8 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.je...@samsung.com>
Date: Fri, 9 Nov 2018 16:08:37 +0900
Subject: [PATCH 4/5] utils: fix coding styles
Signed-off-by: 2xsec <dh48.je...@samsung.com>
---
src/lxc/initutils.c | 2 +-
src/lxc/utils.c | 137 ++++++++++++++++++++++++--------------------
2 files changed, 77 insertions(+), 62 deletions(-)
diff --git a/src/lxc/initutils.c b/src/lxc/initutils.c
index 79333e272..11c808662 100644
--- a/src/lxc/initutils.c
+++ b/src/lxc/initutils.c
@@ -321,7 +321,7 @@ int setproctitle(char *title)
if (ret == 0)
(void)strlcpy((char*)arg_start, title, len);
else
- SYSINFO("setting cmdline failed");
+ SYSWARN("Failed to set cmdline");
return ret;
}
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index be5f3ebe0..1c0baae31 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -83,13 +83,13 @@ static int _recursive_rmdir(const char *dirname, dev_t pdev,
{
struct dirent *direntp;
DIR *dir;
- int ret, failed=0;
+ int ret, failed = 0;
char pathname[PATH_MAX];
bool hadexclude = false;
dir = opendir(dirname);
if (!dir) {
- ERROR("failed to open %s", dirname);
+ ERROR("Failed to open \"%s\"", dirname);
return -1;
}
@@ -103,7 +103,7 @@ static int _recursive_rmdir(const char *dirname, dev_t pdev,
rc = snprintf(pathname, PATH_MAX, "%s/%s", dirname,
direntp->d_name);
if (rc < 0 || rc >= PATH_MAX) {
- ERROR("pathname too long");
+ ERROR("The name of path is too long");
failed=1;
continue;
}
@@ -113,26 +113,27 @@ static int _recursive_rmdir(const char *dirname, dev_t
pdev,
if (ret < 0) {
switch(errno) {
case ENOTEMPTY:
- INFO("Not deleting snapshot %s",
pathname);
+ INFO("Not deleting snapshot \"%s\"",
pathname);
hadexclude = true;
break;
case ENOTDIR:
ret = unlink(pathname);
if (ret)
- INFO("Failed to remove %s",
pathname);
+ INFO("Failed to remove \"%s\"",
pathname);
break;
default:
- SYSERROR("Failed to rmdir %s",
pathname);
+ SYSERROR("Failed to rmdir \"%s\"",
pathname);
failed = 1;
break;
}
}
+
continue;
}
ret = lstat(pathname, &mystat);
if (ret) {
- ERROR("Failed to stat %s", pathname);
+ SYSERROR("Failed to stat \"%s\"", pathname);
failed = 1;
continue;
}
@@ -141,7 +142,7 @@ static int _recursive_rmdir(const char *dirname, dev_t pdev,
/* TODO should we be checking /proc/self/mountinfo for
* pathname and not doing this if found? */
if (btrfs_try_remove_subvol(pathname))
- INFO("Removed btrfs subvolume at %s\n",
pathname);
+ INFO("Removed btrfs subvolume at \"%s\"",
pathname);
continue;
}
@@ -150,20 +151,20 @@ static int _recursive_rmdir(const char *dirname, dev_t
pdev,
failed=1;
} else {
if (unlink(pathname) < 0) {
- SYSERROR("Failed to delete %s", pathname);
+ SYSERROR("Failed to delete \"%s\"", pathname);
failed=1;
}
}
}
if (rmdir(dirname) < 0 && !btrfs_try_remove_subvol(dirname) &&
!hadexclude) {
- ERROR("Failed to delete %s", dirname);
+ SYSERROR("Failed to delete \"%s\"", dirname);
failed=1;
}
ret = closedir(dir);
if (ret) {
- ERROR("Failed to close directory %s", dirname);
+ SYSERROR("Failed to close directory \"%s\"", dirname);
failed=1;
}
@@ -195,7 +196,7 @@ extern int lxc_rmdir_onedev(const char *path, const char
*exclude)
if (errno == ENOENT)
return 0;
- ERROR("Failed to stat %s", path);
+ SYSERROR("Failed to stat \"%s\"", path);
return -1;
}
@@ -225,6 +226,7 @@ int mkdir_p(const char *dir, mode_t mode)
{
const char *tmp = dir;
const char *orig = dir;
+
do {
int ret;
char *makeme;
@@ -243,8 +245,8 @@ int mkdir_p(const char *dir, mode_t mode)
free(makeme);
return -1;
}
- free(makeme);
+ free(makeme);
} while (tmp != dir);
return 0;
@@ -270,10 +272,10 @@ char *get_rundir()
return rundir;
}
- INFO("XDG_RUNTIME_DIR isn't set in the environment.");
+ INFO("XDG_RUNTIME_DIR isn't set in the environment");
homedir = getenv("HOME");
if (!homedir) {
- ERROR("HOME isn't set in the environment.");
+ ERROR("HOME isn't set in the environment");
return NULL;
}
@@ -349,24 +351,24 @@ int sha1sum_file(char *fnam, unsigned char *digest)
f = fopen_cloexec(fnam, "r");
if (!f) {
- SYSERROR("Error opening template");
+ SYSERROR("Failed to open template \"%s\"", fnam);
return -1;
}
if (fseek(f, 0, SEEK_END) < 0) {
- SYSERROR("Error seeking to end of template");
+ SYSERROR("Failed to seek to end of template");
fclose(f);
return -1;
}
if ((flen = ftell(f)) < 0) {
- SYSERROR("Error telling size of template");
+ SYSERROR("Failed to tell size of template");
fclose(f);
return -1;
}
if (fseek(f, 0, SEEK_SET) < 0) {
- SYSERROR("Error seeking to start of template");
+ SYSERROR("Failed to seek to start of template");
fclose(f);
return -1;
}
@@ -378,14 +380,14 @@ int sha1sum_file(char *fnam, unsigned char *digest)
}
if (fread(buf, 1, flen, f) != flen) {
- SYSERROR("Failure reading template");
+ SYSERROR("Failed to read template");
free(buf);
fclose(f);
return -1;
}
if (fclose(f) < 0) {
- SYSERROR("Failre closing template");
+ SYSERROR("Failed to close template");
free(buf);
return -1;
}
@@ -513,17 +515,17 @@ int lxc_pclose(struct lxc_popen_FILE *fp)
int randseed(bool srand_it)
{
+ FILE *f;
/*
- srand pre-seed function based on /dev/urandom
- */
+ * srand pre-seed function based on /dev/urandom
+ */
unsigned int seed = time(NULL) + getpid();
- FILE *f;
f = fopen("/dev/urandom", "r");
if (f) {
int ret = fread(&seed, sizeof(seed), 1, f);
if (ret != 1)
- SYSDEBUG("unable to fread /dev/urandom, fallback to
time+pid rand seed");
+ SYSDEBUG("Unable to fread /dev/urandom, fallback to
time+pid rand seed");
fclose(f);
}
@@ -539,9 +541,13 @@ uid_t get_ns_uid(uid_t orig)
char *line = NULL;
size_t sz = 0;
uid_t nsid, hostid, range;
- FILE *f = fopen("/proc/self/uid_map", "r");
- if (!f)
+ FILE *f;
+
+ f = fopen("/proc/self/uid_map", "r");
+ if (!f) {
+ SYSERROR("Failed to open uid_map");
return 0;
+ }
while (getline(&line, &sz, f) != -1) {
if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3)
@@ -566,9 +572,13 @@ gid_t get_ns_gid(gid_t orig)
char *line = NULL;
size_t sz = 0;
gid_t nsid, hostid, range;
- FILE *f = fopen("/proc/self/gid_map", "r");
- if (!f)
+ FILE *f;
+
+ f = fopen("/proc/self/gid_map", "r");
+ if (!f) {
+ SYSERROR("Failed to open gid_map");
return 0;
+ }
while (getline(&line, &sz, f) != -1) {
if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3)
@@ -610,8 +620,7 @@ uint64_t fnv_64a_buf(void *buf, size_t len, uint64_t hval)
{
unsigned char *bp;
- for(bp = buf; bp < (unsigned char *)buf + len; bp++)
- {
+ for(bp = buf; bp < (unsigned char *)buf + len; bp++) {
/* xor the bottom with the current octet */
hval ^= (uint64_t)*bp;
@@ -672,6 +681,7 @@ int detect_shared_rootfs(void)
{
if (is_shared_mountpoint("/"))
return 1;
+
return 0;
}
@@ -687,13 +697,13 @@ bool switch_to_ns(pid_t pid, const char *ns)
fd = open(nspath, O_RDONLY);
if (fd < 0) {
- SYSERROR("Failed to open %s", nspath);
+ SYSERROR("Failed to open \"%s\"", nspath);
return false;
}
ret = setns(fd, 0);
if (ret) {
- SYSERROR("Failed to set process %d to %s of %d.", pid, ns, fd);
+ SYSERROR("Failed to set process %d to \"%s\" of %d.", pid, ns,
fd);
close(fd);
return false;
}
@@ -718,8 +728,10 @@ bool detect_ramfs_rootfs(void)
int i;
f = fopen("/proc/self/mountinfo", "r");
- if (!f)
+ if (!f) {
+ SYSERROR("Failed to open mountinfo");
return false;
+ }
while (getline(&line, &len, f) != -1) {
for (p = line, i = 0; p && i < 4; i++)
@@ -806,10 +818,9 @@ char *choose_init(const char *rootfs)
retv = on_path("init.lxc", rootfs);
- if (env_set) {
+ if (env_set)
if (unsetenv("PATH"))
SYSERROR("Failed to unsetenv");
- }
if (retv)
return retv;
@@ -825,7 +836,7 @@ char *choose_init(const char *rootfs)
ret = snprintf(retv, PATH_MAX, "%s/%s/%s", tmp, SBINDIR, "/init.lxc");
if (ret < 0 || ret >= PATH_MAX) {
- ERROR("pathname too long");
+ ERROR("The name of path is too long");
goto out1;
}
@@ -834,7 +845,7 @@ char *choose_init(const char *rootfs)
ret = snprintf(retv, PATH_MAX, "%s/%s/%s", tmp, LXCINITDIR,
"/lxc/lxc-init");
if (ret < 0 || ret >= PATH_MAX) {
- ERROR("pathname too long");
+ ERROR("The name of path is too long");
goto out1;
}
@@ -843,7 +854,7 @@ char *choose_init(const char *rootfs)
ret = snprintf(retv, PATH_MAX, "%s/usr/lib/lxc/lxc-init", tmp);
if (ret < 0 || ret >= PATH_MAX) {
- ERROR("pathname too long");
+ ERROR("The name of path is too long");
goto out1;
}
@@ -852,7 +863,7 @@ char *choose_init(const char *rootfs)
ret = snprintf(retv, PATH_MAX, "%s/sbin/lxc-init", tmp);
if (ret < 0 || ret >= PATH_MAX) {
- ERROR("pathname too long");
+ ERROR("The name of path is too long");
goto out1;
}
@@ -941,6 +952,7 @@ static char *get_nextpath(char *path, int *offsetp, int
fulllen)
offset++;
*offsetp = offset;
+
return (offset < fulllen) ? &path[offset] : NULL;
}
@@ -1038,7 +1050,7 @@ static int open_if_safe(int dirfd, const char *nextpath)
static int open_without_symlink(const char *target, const char *prefix_skip)
{
int curlen = 0, dirfd, fulllen, i;
- char *dup = NULL;
+ char *dup;
fulllen = strlen(target);
@@ -1046,8 +1058,8 @@ static int open_without_symlink(const char *target, const
char *prefix_skip)
if (prefix_skip && strlen(prefix_skip) > 0) {
curlen = strlen(prefix_skip);
if (!is_subdir(target, prefix_skip, curlen)) {
- ERROR("WHOA there - target '%s' didn't start with
prefix '%s'",
- target, prefix_skip);
+ ERROR("WHOA there - target \"%s\" didn't start with
prefix \"%s\"",
+ target, prefix_skip);
return -EINVAL;
}
@@ -1065,7 +1077,7 @@ static int open_without_symlink(const char *target, const
char *prefix_skip)
/* Make a copy of target which we can hack up, and tokenize it */
if ((dup = strdup(target)) == NULL) {
- SYSERROR("Out of memory checking for symbolic link");
+ ERROR("Out of memory checking for symbolic link");
return -ENOMEM;
}
@@ -1075,8 +1087,10 @@ static int open_without_symlink(const char *target,
const char *prefix_skip)
}
dirfd = open(prefix_skip, O_RDONLY);
- if (dirfd < 0)
+ if (dirfd < 0) {
+ SYSERROR("Failed to open path \"%s\"", prefix_skip);
goto out;
+ }
while (1) {
int newfd, saved_errno;
@@ -1126,7 +1140,7 @@ int safe_mount(const char *src, const char *dest, const
char *fstype,
/* todo - allow symlinks for relative paths if 'allowsymlinks' option
is passed */
if (flags & MS_BIND && src && src[0] != '/') {
- INFO("this is a relative bind mount");
+ INFO("This is a relative bind mount");
srcfd = open_without_symlink(src, NULL);
if (srcfd < 0)
@@ -1170,7 +1184,7 @@ int safe_mount(const char *src, const char *dest, const
char *fstype,
close(destfd);
if (ret < 0) {
errno = saved_errno;
- SYSERROR("Failed to mount %s onto %s", src ? src : "(null)",
dest);
+ SYSERROR("Failed to mount \"%s\" onto \"%s\"", src ? src :
"(null)", dest);
return ret;
}
@@ -1191,13 +1205,13 @@ int safe_mount(const char *src, const char *dest, const
char *fstype,
*/
int lxc_mount_proc_if_needed(const char *rootfs)
{
- char path[PATH_MAX];
+ char path[PATH_MAX] = {0};
int link_to_pid, linklen, mypid, ret;
char link[INTTYPE_TO_STRLEN(pid_t)] = {0};
ret = snprintf(path, PATH_MAX, "%s/proc/self", rootfs);
if (ret < 0 || ret >= PATH_MAX) {
- SYSERROR("proc path name too long");
+ SYSERROR("The name of proc path is too long");
return -1;
}
@@ -1205,7 +1219,7 @@ int lxc_mount_proc_if_needed(const char *rootfs)
ret = snprintf(path, PATH_MAX, "%s/proc", rootfs);
if (ret < 0 || ret >= PATH_MAX) {
- SYSERROR("proc path name too long");
+ SYSERROR("The name of proc path is too long");
return -1;
}
@@ -1217,7 +1231,7 @@ int lxc_mount_proc_if_needed(const char *rootfs)
goto domount;
} else if (linklen >= sizeof(link)) {
link[linklen - 1] = '\0';
- ERROR("readlink returned truncated content: \"%s\"", link);
+ ERROR("Readlink returned truncated content: \"%s\"", link);
return -1;
}
@@ -1233,7 +1247,7 @@ int lxc_mount_proc_if_needed(const char *rootfs)
ret = umount2(path, MNT_DETACH);
if (ret < 0)
- WARN("failed to umount \"%s\" with MNT_DETACH", path);
+ SYSWARN("Failed to umount \"%s\" with MNT_DETACH", path);
domount:
/* rootfs is NULL */
@@ -1244,14 +1258,13 @@ int lxc_mount_proc_if_needed(const char *rootfs)
if (ret < 0)
return -1;
- INFO("mounted /proc in container for security transition");
+ INFO("Mounted /proc in container for security transition");
return 1;
}
int open_devnull(void)
{
int fd = open("/dev/null", O_RDWR);
-
if (fd < 0)
SYSERROR("Can't open /dev/null");
@@ -1300,7 +1313,7 @@ int null_stdfds(void)
bool task_blocks_signal(pid_t pid, int signal)
{
int ret;
- char status[__PROC_STATUS_LEN];
+ char status[__PROC_STATUS_LEN] = {0};
FILE *f;
uint64_t sigblk = 0, one = 1;
size_t n = 0;
@@ -1560,7 +1573,7 @@ int run_command(char *buf, size_t buf_size, int
(*child_fn)(void *), void *args)
buf[0] = '\0';
if (pipe(pipefd) < 0) {
- SYSERROR("failed to create pipe");
+ SYSERROR("Failed to create pipe");
return -1;
}
@@ -1568,7 +1581,7 @@ int run_command(char *buf, size_t buf_size, int
(*child_fn)(void *), void *args)
if (child < 0) {
close(pipefd[0]);
close(pipefd[1]);
- SYSERROR("failed to create new process");
+ SYSERROR("Failed to create new process");
return -1;
}
@@ -1587,13 +1600,13 @@ int run_command(char *buf, size_t buf_size, int
(*child_fn)(void *), void *args)
close(pipefd[1]);
if (ret < 0) {
- SYSERROR("failed to duplicate std{err,out} file
descriptor");
+ SYSERROR("Failed to duplicate std{err,out} file
descriptor");
_exit(EXIT_FAILURE);
}
/* Does not return. */
child_fn(args);
- ERROR("failed to exec command");
+ ERROR("Failed to exec command");
_exit(EXIT_FAILURE);
}
@@ -1706,8 +1719,10 @@ int recursive_destroy(char *dirname)
int r = 0;
dir = opendir(dirname);
- if (!dir)
+ if (!dir) {
+ SYSERROR("Failed to open dir \"%s\"", dirname);
return -1;
+ }
while ((direntp = readdir(dir))) {
char *pathname;
@@ -1722,7 +1737,7 @@ int recursive_destroy(char *dirname)
ret = lstat(pathname, &mystat);
if (ret < 0) {
if (!r)
- WARN("Failed to stat \"%s\"", pathname);
+ SYSWARN("Failed to stat \"%s\"", pathname);
r = -1;
goto next;
From 2f32e37ef41c97ae9d166457d7b0141df96dc3fd Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.je...@samsung.com>
Date: Fri, 9 Nov 2018 16:10:15 +0900
Subject: [PATCH 5/5] utils: add errno logs for exception case
Signed-off-by: 2xsec <dh48.je...@samsung.com>
---
src/lxc/utils.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 1c0baae31..2f864e3a7 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1418,8 +1418,10 @@ static int lxc_get_unused_loop_dev_legacy(char
*loop_name)
int dfd = -1, fd = -1, ret = -1;
dir = opendir("/dev");
- if (!dir)
+ if (!dir) {
+ SYSERROR("Failed to open \"/dev\"");
return -1;
+ }
while ((dp = readdir(dir))) {
if (strncmp(dp->d_name, "loop", 4) != 0)
@@ -1467,12 +1469,16 @@ static int lxc_get_unused_loop_dev(char *name_loop)
int fd_ctl = -1, fd_tmp = -1;
fd_ctl = open("/dev/loop-control", O_RDWR | O_CLOEXEC);
- if (fd_ctl < 0)
+ if (fd_ctl < 0) {
+ SYSERROR("Failed to open loop control");
return -ENODEV;
+ }
loop_nr = ioctl(fd_ctl, LOOP_CTL_GET_FREE);
- if (loop_nr < 0)
+ if (loop_nr < 0) {
+ SYSERROR("Failed to get loop control");
goto on_error;
+ }
ret = snprintf(name_loop, LO_NAME_SIZE, "/dev/loop%d", loop_nr);
if (ret < 0 || ret >= LO_NAME_SIZE)
@@ -1480,7 +1486,7 @@ static int lxc_get_unused_loop_dev(char *name_loop)
fd_tmp = open(name_loop, O_RDWR | O_CLOEXEC);
if (fd_tmp < 0)
- goto on_error;
+ SYSERROR("Failed to open loop \"%s\"", name_loop);
on_error:
close(fd_ctl);
@@ -1495,26 +1501,34 @@ int lxc_prepare_loop_dev(const char *source, char
*loop_dev, int flags)
fd_loop = lxc_get_unused_loop_dev(loop_dev);
if (fd_loop < 0) {
- if (fd_loop == -ENODEV)
- fd_loop = lxc_get_unused_loop_dev_legacy(loop_dev);
- else
+ if (fd_loop != -ENODEV)
+ goto on_error;
+
+ fd_loop = lxc_get_unused_loop_dev_legacy(loop_dev);
+ if (fd_loop < 0)
goto on_error;
}
fd_img = open(source, O_RDWR | O_CLOEXEC);
- if (fd_img < 0)
+ if (fd_img < 0) {
+ SYSERROR("Failed to open source \"%s\"", source);
goto on_error;
+ }
ret = ioctl(fd_loop, LOOP_SET_FD, fd_img);
- if (ret < 0)
+ if (ret < 0) {
+ SYSERROR("Failed to set loop fd");
goto on_error;
+ }
memset(&lo64, 0, sizeof(lo64));
lo64.lo_flags = flags;
ret = ioctl(fd_loop, LOOP_SET_STATUS64, &lo64);
- if (ret < 0)
+ if (ret < 0) {
+ SYSERROR("Failed to set loop status64");
goto on_error;
+ }
fret = 0;
@@ -1681,10 +1695,8 @@ int lxc_set_death_signal(int signal, pid_t parent)
return -1;
}
- if (ret < 0) {
- SYSERROR("Failed to set PR_SET_PDEATHSIG to %d", signal);
+ if (ret < 0)
return -1;
- }
return 0;
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
