From c63e5b39e855853a0517d5d631714e9114d09b33 Mon Sep 17 00:00:00 2001 From: Robert Swiecki Date: Wed, 10 Aug 2022 15:23:53 +0200 Subject: [PATCH] use QC() across the code --- cgroup.cc | 12 ++++++------ cmdline.cc | 11 +++++------ cpu.cc | 8 +++++--- mnt.cc | 39 +++++++++++++++++++-------------------- subproc.cc | 9 ++++----- util.h | 2 ++ 6 files changed, 41 insertions(+), 40 deletions(-) diff --git a/cgroup.cc b/cgroup.cc index c5ce485..e5df026 100644 --- a/cgroup.cc +++ b/cgroup.cc @@ -38,9 +38,9 @@ namespace cgroup { static bool createCgroup(const std::string& cgroup_path, pid_t pid) { - LOG_D("Create '%s' for pid=%d", cgroup_path.c_str(), (int)pid); + LOG_D("Create %s for pid=%d", QC(cgroup_path), (int)pid); if (mkdir(cgroup_path.c_str(), 0700) == -1 && errno != EEXIST) { - PLOG_W("mkdir('%s', 0700) failed", cgroup_path.c_str()); + PLOG_W("mkdir(%s, 0700) failed", QC(cgroup_path)); return false; } return true; @@ -48,7 +48,7 @@ static bool createCgroup(const std::string& cgroup_path, pid_t pid) { static bool writeToCgroup( const std::string& cgroup_path, const std::string& value, const std::string& what) { - LOG_D("Setting '%s' to '%s'", cgroup_path.c_str(), value.c_str()); + LOG_D("Setting %s to '%s'", QC(cgroup_path), value.c_str()); if (!util::writeBufToFile( cgroup_path.c_str(), value.c_str(), value.length(), O_WRONLY | O_CLOEXEC)) { LOG_W("Could not update %s", what.c_str()); @@ -60,7 +60,7 @@ static bool writeToCgroup( static bool addPidToTaskList(const std::string& cgroup_path, pid_t pid) { std::string pid_str = std::to_string(pid); std::string tasks_path = cgroup_path + "/tasks"; - LOG_D("Adding pid='%s' to '%s'", pid_str.c_str(), tasks_path.c_str()); + LOG_D("Adding pid='%s' to %s", pid_str.c_str(), QC(tasks_path)); return writeToCgroup(tasks_path, pid_str, "'" + tasks_path + "' task list"); } @@ -165,9 +165,9 @@ bool initNsFromParent(nsjconf_t* nsjconf, pid_t pid) { } static void removeCgroup(const std::string& cgroup_path) { - LOG_D("Remove '%s'", cgroup_path.c_str()); + LOG_D("Remove %s", QC(cgroup_path)); if (rmdir(cgroup_path.c_str()) == -1) { - PLOG_W("rmdir('%s') failed", cgroup_path.c_str()); + PLOG_W("rmdir(%s) failed", QC(cgroup_path)); } } diff --git a/cmdline.cc b/cmdline.cc index 0da4df8..5194055 100644 --- a/cmdline.cc +++ b/cmdline.cc @@ -210,8 +210,7 @@ void addEnv(nsjconf_t* nsjconf, const std::string& env) { } char* e = getenv(env.c_str()); if (!e) { - LOG_W("Requested to use the '%s' envar, but it's not set. It'll be ignored", - env.c_str()); + LOG_W("Requested to use the %s envar, but it's not set. It'll be ignored", QC(env)); return; } nsjconf->envs.push_back(std::string(env).append("=").append(e)); @@ -237,13 +236,13 @@ void logParams(nsjconf_t* nsjconf) { } LOG_I( - "Jail parameters: hostname:'%s', chroot:'%s', process:'%s', bind:[%s]:%d, " + "Jail parameters: hostname:'%s', chroot:%s, process:'%s', bind:[%s]:%d, " "max_conns:%u, max_conns_per_ip:%u, time_limit:%" PRId64 ", personality:%#lx, daemonize:%s, clone_newnet:%s, " "clone_newuser:%s, clone_newns:%s, clone_newpid:%s, clone_newipc:%s, clone_newuts:%s, " "clone_newcgroup:%s, clone_newtime:%s, keep_caps:%s, disable_no_new_privs:%s, " "max_cpus:%zu", - nsjconf->hostname.c_str(), nsjconf->chroot.c_str(), + nsjconf->hostname.c_str(), QC(nsjconf->chroot), nsjconf->exec_file.empty() ? nsjconf->argv[0].c_str() : nsjconf->exec_file.c_str(), nsjconf->bindhost.c_str(), nsjconf->port, nsjconf->max_conns, nsjconf->max_conns_per_ip, nsjconf->tlimit, nsjconf->personality, logYesNo(nsjconf->daemonize), @@ -344,7 +343,7 @@ static bool setupArgv(nsjconf_t* nsjconf, int argc, char** argv, int optind) { #endif /* !defined(__NR_execveat) */ if ((nsjconf->exec_fd = TEMP_FAILURE_RETRY( open(nsjconf->exec_file.c_str(), O_RDONLY | O_PATH | O_CLOEXEC))) == -1) { - PLOG_W("Couldn't open '%s' file", nsjconf->exec_file.c_str()); + PLOG_W("Couldn't open %s file", QC(nsjconf->exec_file)); return false; } } @@ -522,7 +521,7 @@ std::unique_ptr parseArgs(int argc, char* argv[]) { break; case 'C': if (!config::parseFile(nsjconf.get(), optarg)) { - LOG_F("Couldn't parse configuration from '%s' file", optarg); + LOG_F("Couldn't parse configuration from %s file", QC(optarg)); } break; case 'c': diff --git a/cpu.cc b/cpu.cc index b98bfb7..cb4b5b5 100644 --- a/cpu.cc +++ b/cpu.cc @@ -70,8 +70,10 @@ static void setRandomCpu(cpu_set_t* orig_mask, cpu_set_t* new_mask, size_t avail n = getNthOnlineCpu(orig_mask, n); CPU_SET(n, new_mask); - LOG_D("Add CPU #%zu from the original set of [%s] (size=%zu), new mask [%s] (size=%zu)", n, - listCpusInSet(orig_mask).c_str(), (size_t)CPU_COUNT(orig_mask), + LOG_D( + "Add CPU #%zu from the original mask=[%s] (size=%zu, available_cpus=%zu), new " + "mask=[%s] (size=%zu)", + n, listCpusInSet(orig_mask).c_str(), (size_t)CPU_COUNT(orig_mask), available_cpus, listCpusInSet(new_mask).c_str(), (size_t)CPU_COUNT(new_mask)); CPU_CLR(n, orig_mask); } @@ -121,7 +123,7 @@ bool initCpu(nsjconf_t* nsjconf) { LOG_D( "Setting new CPU mask=[%s] with %zu allowed CPUs (max_cpus=%zu), %zu CPUs " - "(CPU_COUNT=%zu) left CPU mask=[%s]", + "(CPU_COUNT=%zu) left mask=[%s]", listCpusInSet(new_mask.get()).c_str(), nsjconf->max_cpus, (size_t)CPU_COUNT(new_mask.get()), available_cpus, (size_t)CPU_COUNT(orig_mask.get()), listCpusInSet(orig_mask.get()).c_str()); diff --git a/mnt.cc b/mnt.cc index efca403..dd9b494 100644 --- a/mnt.cc +++ b/mnt.cc @@ -165,15 +165,14 @@ static bool mountPt(mount_t* mpt, const char* newroot, const char* tmpdir) { if (mpt->is_dir) { if (mkdir(dstpath, 0711) == -1 && errno != EEXIST) { - PLOG_W("mkdir(%s)", util::StrQuote(dstpath).c_str()); + PLOG_W("mkdir(%s)", QC(dstpath)); } } else { int fd = TEMP_FAILURE_RETRY(open(dstpath, O_CREAT | O_RDONLY | O_CLOEXEC, 0644)); if (fd >= 0) { close(fd); } else { - PLOG_W("open(%s, O_CREAT|O_RDONLY|O_CLOEXEC, 0644)", - util::StrQuote(dstpath).c_str()); + PLOG_W("open(%s, O_CREAT|O_RDONLY|O_CLOEXEC, 0644)", QC(dstpath)); } } @@ -343,7 +342,7 @@ static std::unique_ptr getDir(nsjconf_t* nsjconf, const char* name) return dir; } - LOG_E("Couldn't create tmp directory of type '%s'", name); + LOG_E("Couldn't create tmp directory of type '%s'", QC(name)); return nullptr; } @@ -356,7 +355,7 @@ static bool initNoCloneNs(nsjconf_t* nsjconf) { return true; } if (chroot(nsjconf->chroot.c_str()) == -1) { - PLOG_E("chroot('%s')", nsjconf->chroot.c_str()); + PLOG_E("chroot('%s')", QC(nsjconf->chroot)); return false; } if (chdir("/") == -1) { @@ -384,7 +383,7 @@ static bool initCloneNs(nsjconf_t* nsjconf) { return false; } if (mount(NULL, destdir->c_str(), "tmpfs", 0, "size=16777216") == -1) { - PLOG_E("mount('%s', 'tmpfs')", destdir->c_str()); + PLOG_E("mount('%s', 'tmpfs')", QC(*destdir)); return false; } @@ -394,19 +393,19 @@ static bool initCloneNs(nsjconf_t* nsjconf) { return false; } if (mount(NULL, tmpdir->c_str(), "tmpfs", 0, "size=16777216") == -1) { - PLOG_E("mount('%s', 'tmpfs')", tmpdir->c_str()); + PLOG_E("mount(%s, 'tmpfs')", QC(*tmpdir)); return false; } for (auto& p : nsjconf->mountpts) { if (!mountPt(&p, destdir->c_str(), tmpdir->c_str()) && p.is_mandatory) { - LOG_E("Couldn't mount '%s'", p.dst.c_str()); + LOG_E("Couldn't mount %s", QC(p.dst)); return false; } } if (umount2(tmpdir->c_str(), MNT_DETACH) == -1) { - PLOG_E("umount2('%s', MNT_DETACH)", tmpdir->c_str()); + PLOG_E("umount2(%s, MNT_DETACH)", QC(*tmpdir)); return false; } @@ -421,7 +420,7 @@ static bool initCloneNs(nsjconf_t* nsjconf) { */ if (util::syscall(__NR_pivot_root, (uintptr_t)destdir->c_str(), (uintptr_t)destdir->c_str()) == -1) { - PLOG_E("pivot_root('%s', '%s')", destdir->c_str(), destdir->c_str()); + PLOG_E("pivot_root(%s, %s)", QC(*destdir), QC(*destdir)); return false; } @@ -451,19 +450,19 @@ static bool initCloneNs(nsjconf_t* nsjconf) { "Use it with care!"); if (chdir(destdir->c_str()) == -1) { - PLOG_E("chdir('%s')", destdir->c_str()); + PLOG_E("chdir(%s)", QC(*destdir)); return false; } /* mount moving the new root on top of '/'. This operation is atomic and doesn't involve un-mounting '/' at any stage */ if (mount(".", "/", NULL, MS_MOVE, NULL) == -1) { - PLOG_E("mount('/', %s, NULL, MS_MOVE, NULL)", destdir->c_str()); + PLOG_E("mount('/', %s, NULL, MS_MOVE, NULL)", QC(*destdir)); return false; } if (chroot(".") == -1) { - PLOG_E("chroot('%s')", destdir->c_str()); + PLOG_E("chroot(%s)", QC(*destdir)); return false; } } @@ -489,7 +488,7 @@ static bool initNsInternal(nsjconf_t* nsjconf) { } if (chdir(nsjconf->cwd.c_str()) == -1) { - PLOG_E("chdir('%s')", nsjconf->cwd.c_str()); + PLOG_E("chdir(%s)", QC(nsjconf->cwd)); return false; } return true; @@ -529,7 +528,7 @@ static bool addMountPt(mount_t* mnt, const std::string& src, const std::string& if (!src_env.empty()) { const char* e = getenv(src_env.c_str()); if (e == NULL) { - LOG_W("No such envar:'%s'", src_env.c_str()); + LOG_W("No such envar:%s", QC(src_env)); return false; } mnt->src = e; @@ -539,7 +538,7 @@ static bool addMountPt(mount_t* mnt, const std::string& src, const std::string& if (!dst_env.empty()) { const char* e = getenv(dst_env.c_str()); if (e == NULL) { - LOG_W("No such envar:'%s'", dst_env.c_str()); + LOG_W("No such envar:%s", QC(dst_env)); return false; } mnt->dst = e; @@ -609,15 +608,15 @@ bool addMountPtTail(nsjconf_t* nsjconf, const std::string& src, const std::strin const std::string describeMountPt(const mount_t& mpt) { std::string descr; - descr.append(mpt.src.empty() ? "" : util::StrQuote(mpt.src)) + descr.append(mpt.src.empty() ? "" : QC(mpt.src)) .append(mpt.src.empty() ? "" : " -> ") - .append(util::StrQuote(mpt.dst)) + .append(QC(mpt.dst)) .append(" flags:") .append(flagsToStr(mpt.flags)) .append(" type:") - .append(util::StrQuote(mpt.fs_type)) + .append(QC(mpt.fs_type)) .append(" options:") - .append(util::StrQuote(mpt.options)); + .append(QC(mpt.options)); if (mpt.is_dir) { descr.append(" dir:true"); diff --git a/subproc.cc b/subproc.cc index 69ffa2e..a4e041d 100644 --- a/subproc.cc +++ b/subproc.cc @@ -152,8 +152,7 @@ static const std::string concatArgs(const std::vector& argv) { return ret; } -static void subprocNewProc( - nsjconf_t* nsjconf, int netfd, int fd_in, int fd_out, int fd_err, int pipefd) { +static void newProc(nsjconf_t* nsjconf, int netfd, int fd_in, int fd_out, int fd_err, int pipefd) { if (!contain::setupFD(nsjconf, fd_in, fd_out, fd_err)) { return; } @@ -241,7 +240,7 @@ static void addProc(nsjconf_t* nsjconf, pid_t pid, int sock) { } nsjconf->pids.insert(std::make_pair(pid, p)); - LOG_D("Added pid=%d with start time '%u' to the queue for IP: '%s'", pid, + LOG_D("Added pid=%d with start time %u to the queue for IP: '%s'", pid, (unsigned int)p.start, p.remote_txt.c_str()); } @@ -461,7 +460,7 @@ pid_t runChild(nsjconf_t* nsjconf, int netfd, int fd_in, int fd_out, int fd_err) if (unshare(flags) == -1) { PLOG_F("unshare(%s)", cloneFlagsToStr(flags).c_str()); } - subprocNewProc(nsjconf, netfd, fd_in, fd_out, fd_err, -1); + newProc(nsjconf, netfd, fd_in, fd_out, fd_err, -1); LOG_F("Launching new process failed"); } @@ -479,7 +478,7 @@ pid_t runChild(nsjconf_t* nsjconf, int netfd, int fd_in, int fd_out, int fd_err) pid_t pid = cloneProc(flags, SIGCHLD); if (pid == 0) { close(parent_fd); - subprocNewProc(nsjconf, netfd, fd_in, fd_out, fd_err, child_fd); + newProc(nsjconf, netfd, fd_in, fd_out, fd_err, child_fd); util::writeToFd(child_fd, &kSubprocErrorChar, sizeof(kSubprocErrorChar)); LOG_F("Launching child process failed"); } diff --git a/util.h b/util.h index f2add5a..d3be2e9 100644 --- a/util.h +++ b/util.h @@ -39,6 +39,8 @@ } \ } while (0) +#define QC(x) util::StrQuote(x).c_str() + namespace util { ssize_t readFromFd(int fd, void* buf, size_t len);