diff --git a/CLAUDE.md b/CLAUDE.md index 69a0469..98985ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -481,6 +481,18 @@ Build directory is `buildDir/` (already configured). ## Code style - Null-pointer checks: prefer `if (!ptr)` / `if (ptr)` over `if (ptr == nullptr)` / `if (ptr != nullptr)`. +- Constants: no `k` Hungarian-notation prefix. `enum class` values are already qualified by + the enum's own name (e.g. `Mode::run`, `OciPortProtocol::tcp`), so plain snake_case + enumerators are enough on their own. Free-standing constants also use plain snake_case; + when several are conceptually related, group them under a named `namespace` instead of + relying on a shared prefix to imply the grouping (e.g. `main.cpp`'s `getopt_long` long-option + codes live in `namespace options { constexpr int log_level = ...; }`, and `bwrap.cpp`'s + priv-drop-helper path/binary-name pair live in `namespace priv_drop { ... }`) — nest the named + namespace inside the file's existing anonymous namespace where one is already present, so + internal linkage is unchanged. A `kXxx`-named identifier that turns out not to actually be + `const` (mutable global/static state) instead follows this codebase's existing `g_` prefix + convention (e.g. `containers_storage.cpp`'s `g_mount_program`, matching `process.cpp`'s + `g_foreground_child_pid` and `daemonize.cpp`'s `g_report_fd`/`g_log_path`). ## Licensing diff --git a/src/bwrap.cpp b/src/bwrap.cpp index 55cd7c0..1d47350 100644 --- a/src/bwrap.cpp +++ b/src/bwrap.cpp @@ -37,16 +37,18 @@ namespace { +namespace priv_drop { // Hidden path inside the sandbox where the priv-drop helper binary is bind-mounted // when dropping privileges to a --user/--group (see build_bwrap_args()). -constexpr const char* kPrivDropPath = "/.slocker-lite-priv-drop"; +constexpr const char* path = "/.slocker-lite-priv-drop"; // Name of the statically-linked helper binary built alongside slocker-lite // (src/priv_drop_helper.cpp) -- it has to be a separate, dependency-free static // binary rather than slocker-lite's own binary, since bind-mounting a dynamically // linked executable into an arbitrary container image fails ("error while loading // shared libraries") when that image's own /lib lacks slocker-lite's dependencies. -constexpr const char* kPrivDropHelperName = "slocker-lite-priv-drop"; +constexpr const char* helper_name = "slocker-lite-priv-drop"; +} // namespace priv_drop // Locates the priv-drop helper installed next to this process's own binary (found // via /proc/self/exe), which holds whether run from buildDir/ or after a proper @@ -57,7 +59,7 @@ std::optional find_priv_drop_helper() { if (ec) { return std::nullopt; } - std::filesystem::path candidate = self_path.parent_path() / kPrivDropHelperName; + std::filesystem::path candidate = self_path.parent_path() / priv_drop::helper_name; if (access(candidate.c_str(), X_OK) != 0) { return std::nullopt; } @@ -99,7 +101,7 @@ struct NamespaceProbe { const char* name; }; -constexpr std::array kNamespaceProbes = {{ +constexpr std::array namespace_probes = {{ {CLONE_NEWUSER, "--unshare-user", "user"}, {CLONE_NEWIPC, "--unshare-ipc", "ipc"}, {CLONE_NEWPID, "--unshare-pid", "pid"}, @@ -188,7 +190,7 @@ std::vector detect_bwrap_unshare_args() { args.push_back("--unshare-user"); } - for (const auto& probe : kNamespaceProbes) { + for (const auto& probe : namespace_probes) { if (probe.clone_flag == CLONE_NEWUSER) { continue; } @@ -281,19 +283,19 @@ std::vector build_bwrap_args(const std::string& root, if (user) { auto helper_path = find_priv_drop_helper(); if (helper_path) { - args.insert(args.end(), {"--ro-bind", helper_path->string(), kPrivDropPath}); + args.insert(args.end(), {"--ro-bind", helper_path->string(), priv_drop::path}); bound_priv_drop_helper = true; } else { spdlog::warn("could not find the {} helper next to this binary; --user/--group will " "have no effect", - kPrivDropHelperName); + priv_drop::helper_name); } } args.push_back("--"); if (bound_priv_drop_helper) { - args.push_back(kPrivDropPath); + args.push_back(priv_drop::path); args.push_back(fmt::format("{}:{}", user->uid, user->gid)); args.push_back("--"); } @@ -337,7 +339,7 @@ int run_bwrap(const std::string& root, const std::vector& command, const std::function& on_bwrap_pid_known) { if (user && !find_priv_drop_helper()) { spdlog::error("could not find the {} helper next to this binary; --user/--group requires it", - kPrivDropHelperName); + priv_drop::helper_name); return -1; } diff --git a/src/containers_storage.cpp b/src/containers_storage.cpp index 03f3ae7..8f6a0cc 100644 --- a/src/containers_storage.cpp +++ b/src/containers_storage.cpp @@ -36,13 +36,13 @@ std::string trim(const std::string& s) { } // namespace -std::string kMountProgram; +std::string g_mount_program; std::optional import_layer(const std::filesystem::path& diff_file, const std::string& parent_id) { std::vector argv = { "containers-storage", "import-layer", "--file", diff_file.string(), - "--storage-opt", "overlay.mount_program=" + kMountProgram}; + "--storage-opt", "overlay.mount_program=" + g_mount_program}; if (!parent_id.empty()) { argv.push_back(parent_id); } @@ -58,7 +58,7 @@ std::optional import_layer(const std::filesystem::path& diff_file, std::optional mount_layer(const std::string& layer_id) { std::vector argv = { "containers-storage", "mount", "--storage-opt", - "overlay.mount_program=" + kMountProgram, layer_id}; + "overlay.mount_program=" + g_mount_program, layer_id}; ProcessResult result = run_process(argv); std::string path = trim(result.stdout_output); diff --git a/src/containers_storage.h b/src/containers_storage.h index ef70d57..90d7eb6 100644 --- a/src/containers_storage.h +++ b/src/containers_storage.h @@ -23,7 +23,7 @@ // Absolute path to the fuse-overlayfs binary, passed to containers-storage as // overlay.mount_program so it performs the actual overlay mounting. Set once in main() // after find_in_path() resolves it. -extern std::string kMountProgram; +extern std::string g_mount_program; // Runs `containers-storage import-layer --storage-opt overlay.mount_program= // [--file diff_file] [parent_id]`. Returns the new layer's ID (trimmed stdout) or diff --git a/src/exec_session.cpp b/src/exec_session.cpp index a9345bc..a415704 100644 --- a/src/exec_session.cpp +++ b/src/exec_session.cpp @@ -62,7 +62,7 @@ struct JoinableNamespace { // net is deliberately excluded: this project never isolates networking either // (see build_bwrap_args() dropping --unshare-net), so there's nothing meaningful // to join there. -constexpr std::array kJoinableNamespaces = {{ +constexpr std::array joinable_namespaces = {{ {"mnt", "--mount", true}, {"uts", "--uts", false}, {"ipc", "--ipc", false}, @@ -103,7 +103,7 @@ int exec_in_session(pid_t pid, const std::vector& command) { pid_t ns_pid = resolve_namespace_pid(pid); std::vector argv = {"nsenter"}; - for (const auto& ns : kJoinableNamespaces) { + for (const auto& ns : joinable_namespaces) { auto target_ns = read_ns_link(fmt::format("/proc/{}/ns/{}", ns_pid, ns.proc_name)); if (!target_ns) { if (ns.required) { diff --git a/src/main.cpp b/src/main.cpp index 385e5b6..00f70ab 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -46,24 +46,29 @@ namespace { -constexpr std::array kRequiredTools = {"containers-storage", "bwrap"}; +constexpr std::array required_tools = {"containers-storage", "bwrap"}; + +// Shared by list_images_command()/list_volumes_command()/list_processes_command(): +// pad each entry with tabs (not spaces) so columns line up on an 8-column tab +// stop past the longest entry in that column, however long that is. +constexpr size_t tab_width = 8; enum class Mode { - kNone, - kMount, - kUnmount, - kTest, - kRun, - kCleanup, - kListImages, - kVolume, - kListVolumes, - kDeleteVolume, - kDeleteVolumeFull, - kInspect, - kListProcesses, - kCleanProcesses, - kExec + none, + mount, + unmount, + test, + run, + cleanup, + list_images, + volume, + list_volumes, + delete_volume, + delete_volume_full, + inspect, + list_processes, + clean_processes, + exec }; // --log-level/--user/--group/--list-volumes/--delete-volume[-full]/--hostname/ @@ -71,23 +76,25 @@ enum class Mode { // (--log-level's was freed up so -l could become --list-images; -u is already // --umount; the rest have no natural free letter left, or don't need one), so // they need long-option vals outside the printable-char range short options use. -constexpr int kLogLevelOpt = 256; -constexpr int kUserOpt = 257; -constexpr int kGroupOpt = 258; -constexpr int kListVolumesOpt = 259; -constexpr int kDeleteVolumeOpt = 260; -constexpr int kDeleteVolumeFullOpt = 261; -constexpr int kHostnameOpt = 262; -constexpr int kListProcessesOpt = 263; -constexpr int kCleanProcessesOpt = 264; -constexpr int kEnvOpt = 265; -constexpr int kEnvFileOpt = 266; +namespace options { +constexpr int log_level = 256; +constexpr int user = 257; +constexpr int group = 258; +constexpr int list_volumes = 259; +constexpr int delete_volume = 260; +constexpr int delete_volume_full = 261; +constexpr int hostname = 262; +constexpr int list_processes = 263; +constexpr int clean_processes = 264; +constexpr int env = 265; +constexpr int env_file = 266; +} // namespace options -constexpr std::array kLongOptions = {{ +constexpr std::array long_options = {{ {"help", no_argument, nullptr, 'h'}, {"version", no_argument, nullptr, 'V'}, {"test", no_argument, nullptr, 't'}, - {"log-level", required_argument, nullptr, kLogLevelOpt}, + {"log-level", required_argument, nullptr, options::log_level}, {"mount", required_argument, nullptr, 'm'}, {"umount", required_argument, nullptr, 'u'}, {"run", required_argument, nullptr, 'r'}, @@ -95,19 +102,19 @@ constexpr std::array kLongOptions = {{ {"no-nsenter", no_argument, nullptr, 'n'}, {"daemonize", no_argument, nullptr, 'D'}, {"list-images", required_argument, nullptr, 'l'}, - {"user", required_argument, nullptr, kUserOpt}, - {"group", required_argument, nullptr, kGroupOpt}, + {"user", required_argument, nullptr, options::user}, + {"group", required_argument, nullptr, options::group}, {"volume", required_argument, nullptr, 'v'}, - {"list-volumes", no_argument, nullptr, kListVolumesOpt}, - {"delete-volume", required_argument, nullptr, kDeleteVolumeOpt}, - {"delete-volume-full", required_argument, nullptr, kDeleteVolumeFullOpt}, + {"list-volumes", no_argument, nullptr, options::list_volumes}, + {"delete-volume", required_argument, nullptr, options::delete_volume}, + {"delete-volume-full", required_argument, nullptr, options::delete_volume_full}, {"inspect", required_argument, nullptr, 'i'}, {"exec", required_argument, nullptr, 'e'}, - {"hostname", required_argument, nullptr, kHostnameOpt}, - {"list-processes", no_argument, nullptr, kListProcessesOpt}, - {"clean-processes", no_argument, nullptr, kCleanProcessesOpt}, - {"env", required_argument, nullptr, kEnvOpt}, - {"env-file", required_argument, nullptr, kEnvFileOpt}, + {"hostname", required_argument, nullptr, options::hostname}, + {"list-processes", no_argument, nullptr, options::list_processes}, + {"clean-processes", no_argument, nullptr, options::clean_processes}, + {"env", required_argument, nullptr, options::env}, + {"env-file", required_argument, nullptr, options::env_file}, {nullptr, 0, nullptr, 0}, }}; @@ -228,9 +235,9 @@ void print_version() { } bool apply_log_level(std::string_view name) { - constexpr std::array kValidLevels = { + constexpr std::array valid_levels = { "trace", "debug", "info", "warn", "error", "critical", "off"}; - for (auto level : kValidLevels) { + for (auto level : valid_levels) { if (level == name) { spdlog::set_level(spdlog::level::from_str(std::string(name))); return true; @@ -251,7 +258,7 @@ int run_tests() { bool check_required_dependencies() { bool all_found = true; - for (auto name : kRequiredTools) { + for (auto name : required_tools) { if (!find_in_path(name)) { spdlog::error("required dependency not found in PATH: {}", name); all_found = false; @@ -287,7 +294,7 @@ std::optional mount_image(const std::filesystem::path& image_tar) spdlog::error("fuse-overlayfs not found in PATH"); return std::nullopt; } - kMountProgram = mount_program->string(); + g_mount_program = mount_program->string(); auto layers = read_oci_layers(image_tar); if (!layers) { @@ -348,13 +355,10 @@ int list_images_command(const std::filesystem::path& dir) { max_len = std::max(max_len, refs.back().size()); } - // Pad each entry with tabs (not spaces) so filenames line up on an 8-column tab - // stop past the longest name:tag, however long that is. - constexpr size_t kTabWidth = 8; - size_t target_tabs = max_len / kTabWidth + 1; + size_t target_tabs = max_len / tab_width + 1; for (size_t i = 0; i < images->size(); ++i) { - size_t tabs_used = refs[i].size() / kTabWidth; + size_t tabs_used = refs[i].size() / tab_width; size_t tabs_needed = target_tabs > tabs_used ? target_tabs - tabs_used : 1; fmt::print("{}{}{}\n", refs[i], std::string(tabs_needed, '\t'), (*images)[i].path.filename().string()); @@ -388,7 +392,7 @@ int inspect_image_command(const std::filesystem::path& image_tar) { } else { fmt::print("Exposed ports:\n"); for (const auto& port : config->exposed_ports) { - fmt::print(" {}/{}\n", port.port, port.protocol == OciPortProtocol::kTcp ? "tcp" : "udp"); + fmt::print(" {}/{}\n", port.port, port.protocol == OciPortProtocol::tcp ? "tcp" : "udp"); } } @@ -463,13 +467,10 @@ int list_volumes_command(const AppConfig& config) { max_len = std::max(max_len, volume.name.size()); } - // Same tab-alignment scheme as list_images_command(): pad each name to one - // 8-column tab stop past the longest one, however long that is. - constexpr size_t kTabWidth = 8; - size_t target_tabs = max_len / kTabWidth + 1; + size_t target_tabs = max_len / tab_width + 1; for (const auto& volume : config.volumes) { - size_t tabs_used = volume.name.size() / kTabWidth; + size_t tabs_used = volume.name.size() / tab_width; size_t tabs_needed = target_tabs > tabs_used ? target_tabs - tabs_used : 1; fmt::print("{}{}{}\n", volume.name, std::string(tabs_needed, '\t'), volume.directory); } @@ -492,16 +493,14 @@ int list_processes_command() { max_name_len = std::max(max_name_len, names.back().size()); } - // Same tab-alignment scheme as list_images_command()/list_volumes_command(), - // applied independently to each of the two variable-width columns. - constexpr size_t kTabWidth = 8; - size_t pid_target_tabs = max_pid_len / kTabWidth + 1; - size_t name_target_tabs = max_name_len / kTabWidth + 1; + // Applied independently to each of the two variable-width columns. + size_t pid_target_tabs = max_pid_len / tab_width + 1; + size_t name_target_tabs = max_name_len / tab_width + 1; for (size_t i = 0; i < sessions.size(); ++i) { - size_t pid_tabs_used = pids[i].size() / kTabWidth; + size_t pid_tabs_used = pids[i].size() / tab_width; size_t pid_tabs_needed = pid_target_tabs > pid_tabs_used ? pid_target_tabs - pid_tabs_used : 1; - size_t name_tabs_used = names[i].size() / kTabWidth; + size_t name_tabs_used = names[i].size() / tab_width; size_t name_tabs_needed = name_target_tabs > name_tabs_used ? name_target_tabs - name_tabs_used : 1; fmt::print("{}{}{}{}{}\n", pids[i], std::string(pid_tabs_needed, '\t'), names[i], std::string(name_tabs_needed, '\t'), sessions[i].running ? "running" : "exited"); @@ -677,7 +676,7 @@ int main(int argc, char* argv[]) { apply_log_level(*config->log_level); } - Mode mode = Mode::kNone; + Mode mode = Mode::none; std::string mode_arg; bool disable_nsenter = false; bool daemonize_flag = false; @@ -689,7 +688,7 @@ int main(int argc, char* argv[]) { opterr = 0; int opt; - while ((opt = getopt_long(argc, argv, ":hVtm:u:r:c:nl:v:i:e:D", kLongOptions.data(), nullptr)) != -1) { + while ((opt = getopt_long(argc, argv, ":hVtm:u:r:c:nl:v:i:e:D", long_options.data(), nullptr)) != -1) { switch (opt) { case 'h': print_usage(argv[0]); @@ -705,54 +704,54 @@ int main(int argc, char* argv[]) { case 'l': case 'i': case 'e': - case kListVolumesOpt: - case kDeleteVolumeOpt: - case kDeleteVolumeFullOpt: - case kListProcessesOpt: - case kCleanProcessesOpt: { + case options::list_volumes: + case options::delete_volume: + case options::delete_volume_full: + case options::list_processes: + case options::clean_processes: { Mode requested; switch (opt) { case 't': - requested = Mode::kTest; + requested = Mode::test; break; case 'm': - requested = Mode::kMount; + requested = Mode::mount; break; case 'u': - requested = Mode::kUnmount; + requested = Mode::unmount; break; case 'r': - requested = Mode::kRun; + requested = Mode::run; break; case 'c': - requested = Mode::kCleanup; + requested = Mode::cleanup; break; case 'l': - requested = Mode::kListImages; + requested = Mode::list_images; break; case 'i': - requested = Mode::kInspect; + requested = Mode::inspect; break; case 'e': - requested = Mode::kExec; + requested = Mode::exec; break; - case kListVolumesOpt: - requested = Mode::kListVolumes; + case options::list_volumes: + requested = Mode::list_volumes; break; - case kDeleteVolumeOpt: - requested = Mode::kDeleteVolume; + case options::delete_volume: + requested = Mode::delete_volume; break; - case kDeleteVolumeFullOpt: - requested = Mode::kDeleteVolumeFull; + case options::delete_volume_full: + requested = Mode::delete_volume_full; break; - case kListProcessesOpt: - requested = Mode::kListProcesses; + case options::list_processes: + requested = Mode::list_processes; break; default: - requested = Mode::kCleanProcesses; + requested = Mode::clean_processes; break; } - if (mode != Mode::kNone && mode != requested) { + if (mode != Mode::none && mode != requested) { spdlog::error("multiple actions specified"); print_usage(argv[0]); return 1; @@ -784,24 +783,24 @@ int main(int argc, char* argv[]) { case 'D': daemonize_flag = true; break; - case kLogLevelOpt: + case options::log_level: if (!apply_log_level(optarg)) { return 1; } break; - case kUserOpt: + case options::user: user_flag = optarg; break; - case kGroupOpt: + case options::group: group_flag = optarg; break; - case kHostnameOpt: + case options::hostname: hostname_flag = optarg; break; - case kEnvOpt: + case options::env: env_specs.push_back({false, optarg}); break; - case kEnvFileOpt: + case options::env_file: env_specs.push_back({true, optarg}); break; case ':': @@ -816,8 +815,8 @@ int main(int argc, char* argv[]) { } } - if (!volume_specs.empty() && mode != Mode::kRun) { - if (mode != Mode::kNone) { + if (!volume_specs.empty() && mode != Mode::run) { + if (mode != Mode::none) { spdlog::error("--volume can only be used standalone or together with --run"); print_usage(argv[0]); return 1; @@ -827,14 +826,14 @@ int main(int argc, char* argv[]) { print_usage(argv[0]); return 1; } - mode = Mode::kVolume; + mode = Mode::volume; } - if (mode == Mode::kNone) { + if (mode == Mode::none) { print_usage(argv[0]); return 1; } - if (mode != Mode::kRun && mode != Mode::kExec && optind != argc) { + if (mode != Mode::run && mode != Mode::exec && optind != argc) { print_usage(argv[0]); return 1; } @@ -844,38 +843,38 @@ int main(int argc, char* argv[]) { return 1; } - if (mode == Mode::kTest) { + if (mode == Mode::test) { return run_tests(); } - if (mode == Mode::kUnmount) { + if (mode == Mode::unmount) { return unmount_image(mode_arg); } - if (mode == Mode::kCleanup) { + if (mode == Mode::cleanup) { return cleanup_image(mode_arg); } - if (mode == Mode::kListImages) { + if (mode == Mode::list_images) { return list_images_command(mode_arg); } - if (mode == Mode::kInspect) { + if (mode == Mode::inspect) { return inspect_image_command(mode_arg); } - if (mode == Mode::kVolume) { + if (mode == Mode::volume) { return create_volume_command(volume_specs.front().first, volume_specs.front().second, config_path, *config); } - if (mode == Mode::kListVolumes) { + if (mode == Mode::list_volumes) { return list_volumes_command(*config); } - if (mode == Mode::kDeleteVolume || mode == Mode::kDeleteVolumeFull) { - return delete_volume_command(mode_arg, config_path, *config, mode == Mode::kDeleteVolumeFull); + if (mode == Mode::delete_volume || mode == Mode::delete_volume_full) { + return delete_volume_command(mode_arg, config_path, *config, mode == Mode::delete_volume_full); } - if (mode == Mode::kListProcesses) { + if (mode == Mode::list_processes) { return list_processes_command(); } - if (mode == Mode::kCleanProcesses) { + if (mode == Mode::clean_processes) { return clean_processes_command(); } - if (mode == Mode::kExec) { + if (mode == Mode::exec) { std::vector command(argv + optind, argv + argc); if (command.empty()) { spdlog::error("--exec requires a command to run"); @@ -890,7 +889,7 @@ int main(int argc, char* argv[]) { } return exec_in_session(static_cast(parsed), command); } - if (mode == Mode::kRun) { + if (mode == Mode::run) { std::vector command(argv + optind, argv + argc); // As root, containers-storage mount doesn't need to reexec into a private // user namespace to gain privilege, so the mount is already directly diff --git a/src/oci_image.cpp b/src/oci_image.cpp index d36a73b..605d6aa 100644 --- a/src/oci_image.cpp +++ b/src/oci_image.cpp @@ -220,9 +220,9 @@ std::optional parse_exposed_port(const std::string& key) { OciPortProtocol protocol; if (proto_str == "tcp") { - protocol = OciPortProtocol::kTcp; + protocol = OciPortProtocol::tcp; } else if (proto_str == "udp") { - protocol = OciPortProtocol::kUdp; + protocol = OciPortProtocol::udp; } else { spdlog::debug("skipping exposed port with unrecognized protocol: {}", key); return std::nullopt; diff --git a/src/oci_image.h b/src/oci_image.h index 52d2892..471a26e 100644 --- a/src/oci_image.h +++ b/src/oci_image.h @@ -66,7 +66,7 @@ std::optional read_image_ref(const std::filesystem::path& tar_path) // isn't a readable directory; an empty vector is a valid result (nothing matched). std::optional> list_oci_images(const std::filesystem::path& dir); -enum class OciPortProtocol { kTcp, kUdp }; +enum class OciPortProtocol { tcp, udp }; struct OciExposedPort { int port; diff --git a/src/volume_mount.cpp b/src/volume_mount.cpp index cacfd7e..0703c7c 100644 --- a/src/volume_mount.cpp +++ b/src/volume_mount.cpp @@ -35,11 +35,11 @@ namespace { // setting and immediately removing a throwaway one. Linux POSIX ACLs are // themselves stored as xattrs, so this one probe covers both. bool xattr_supported(const std::filesystem::path& dir) { - constexpr const char* kProbeName = "user.slocker-lite.probe"; - if (setxattr(dir.c_str(), kProbeName, "1", 1, 0) != 0) { + constexpr const char* probe_name = "user.slocker-lite.probe"; + if (setxattr(dir.c_str(), probe_name, "1", 1, 0) != 0) { return false; } - removexattr(dir.c_str(), kProbeName); + removexattr(dir.c_str(), probe_name); return true; }