docs: update coding style a bit

Say that r should be declared at the top of the function.

Don't say that fixed buffers result in truncation, right after saying that they
must only be used if size is known.

Adjust order of examples to be consistent.

TODO

@@ -49,11 +49,6 @@ Features:
 * nspawn: make --bind= work sanely with --private-users when uid mapping mounts
   are used.
 
-* cryptsetup: tweak tpm2-device=auto logic, abort quickly if firmware tells us
-  there isn't any TPM2 device anyway. that way, we'll wait for the TPM2 device
-  to show up only if registered in LUKS header + the firmware suggests there is
-  a device worth waiting for.
-
 * systemd-sysext: optionally, run it in initrd already, before transitioning
   into host, to open up possibility for services shipped like that.
 
@@ -102,20 +97,6 @@ Features:
 
 * move multiseat vid/pid matches from logind udev rule to hwdb
 
-* nspawn: default to 1:1 userns
-
-* Provide a reasonably bespoke solution for mounting host $HOME directories
-  into containers:
-  • add new option --mount-user=$USER for mounting $HOME of the user into the
-    container at the same place
-  • check /etc/passwd for UID or user name clashes. If UID clash pick a different
-    UID in container, and map via userns. If user name clash, refuse. If
-    matching user already exists use that.
-  • otherwise: write user record of specified user into /run/host/passwd or so
-  • in nss-systemd pick up user record from there and make available to system
-  With all that in place if nspawn host and container payload are up-to-date
-  enough we have a very simple way to make host users available in containers.
-
 * whenever we receive fds via SCM_RIGHTS make sure none got dropped due to the
   reception limit the kernel silently enforces.
 
@@ -242,8 +223,6 @@ Features:
 * homed: keep an fd to the homedir open at all times, to keep the fs pinned
   (autofs and such) while user is logged in.
 
-* nss-systemd: also synthesize shadow records for users/groups
-
 * make use of new glibc 2.32 APIs sigabbrev_np() and strerrorname_np().
 
 * when main nspawn supervisor process gets suspended due to SIGSTOP/SIGTTOU or
@@ -456,9 +435,6 @@ Features:
   shouldn't operate in a volatile mode unless we got told so from a trusted
   source.
 
-* figure out automatic partition discovery when combining writable root dir
-  with immutable /usr
-
 * coredump: maybe when coredumping read a new xattr from /proc/$PID/exe that
   may be used to mark a whole binary as non-coredumpable. Would fix:
   https://bugs.freedesktop.org/show_bug.cgi?id=69447
@@ -579,10 +555,6 @@ Features:
   a seccomp option we don't have to set NNP. For that, change uid first whil
   keeping CAP_SYS_ADMIN, then apply seccomp, the drop cap.
 
-* add a concept for automatically loading per-unit secrets off disk and
-  inserting them into the kernel keyring. Maybe SecretsDirectory= similar to
-  ConfigurationDirectory=.
-
 * when no locale is configured, default to UEFI's PlatformLang variable
 
 * bootctl,sd-boot: actually honour the "architecture" key
@@ -635,13 +607,6 @@ Features:
   output of "systemctl list-units" slightly by showing the tree structure of
   the slices, and the units attached to them.
 
-* the a-posteriori stopping of units bound to units that disappeared logic
-  should be reworked: there should be a queue of units, and we should only
-  enqueue stop jobs from a defer event that processes queue instead of
-  right-away when we find a unit that is bound to one that doesn't exist
-  anymore. (similar to how the stop-unneeded queue has been reworked the same
-  way)
-
 * nspawn: make nspawn suitable for shell pipelines: instead of triggering a
   hangup when input is finished, send ^D, which synthesizes an EOF. Then wait
   for hangup or ^D before passing on the EOF.
@@ -1403,7 +1368,6 @@ Features:
     https://bugzilla.redhat.com/show_bug.cgi?id=723942
   - allow writing multiple conditions in unit files on one line
   - introduce Type=pid-file
-  - introduce mix of BindTo and Requisite
   - add a concept of RemainAfterExit= to scope units
   - Allow multiple ExecStart= for all Type= settings, so that we can cover rescue.service nicely
   - add verification of [Install] section to systemd-analyze verify

docs/CODING_STYLE.md

@@ -143,18 +143,37 @@ layout: default
 
 ## Using C Constructs
 
-- Preferably allocate local variables on the top of the block:
+- Allocate local variables where it makes sense: at the top of the block, or at
+  the point where they can be initialized. `r` is typically used for a local
+  state variable, but should almost always be declared at the top of the
+  function.
 
   ```c
   {
-          int a, b;
+          uint64_t a, b;
+          int r;
 
-          a = 5;
+          a = frobnicate();
-          b = a;
+          b = a + 5;
+
+          r = do_something();
+          if (r < 0)
+                  …
   }
   ```
 
-- Do not mix function invocations with variable definitions in one line. Wrong:
+- Do not mix function invocations with variable definitions in one line.
+
+  ```c
+  {
+          uint64_t x = 7;
+          int a;
+
+          a = foobar();
+  }
+  ```
+
+  instead of:
 
   ```c
   {
@@ -163,18 +182,7 @@ layout: default
   }
   ```
 
-  Right:
+- Use `goto` for cleaning up, and only use it for that. I.e. you may only jump
-
-  ```c
-  {
-          int a;
-          uint64_t x = 7;
-
-          a = foobar();
-  }
-  ```
-
-- Use `goto` for cleaning up, and only use it for that. i.e. you may only jump
   to the end of a function, and little else. Never jump backwards!
 
 - To minimize strict aliasing violations, we prefer unions over casting.
@@ -347,8 +355,7 @@ layout: default
   `log_oom()` for then printing a short message, but not in "library" code.
 
 - Avoid fixed-size string buffers, unless you really know the maximum size and
-  that maximum size is small. They are a source of errors, since they possibly
+  that maximum size is small. It is often nicer to use dynamic memory,
-  result in truncated strings. It is often nicer to use dynamic memory,
   `alloca()` or VLAs. If you do allocate fixed-size strings on the stack, then
   it is probably only OK if you either use a maximum size such as `LINE_MAX`,
   or count in detail the maximum size a string can have. (`DECIMAL_STR_MAX` and

src/test/test-stat-util.c

@@ -6,12 +6,14 @@
 #include <unistd.h>
 
 #include "alloc-util.h"
+#include "errno-list.h"
 #include "fd-util.h"
 #include "macro.h"
 #include "mountpoint-util.h"
 #include "namespace-util.h"
 #include "path-util.h"
 #include "stat-util.h"
+#include "tests.h"
 #include "tmpfile-util.h"
 
 static void test_files_same(void) {
@@ -19,6 +21,8 @@ static void test_files_same(void) {
         char name[] = "/tmp/test-files_same.XXXXXX";
         char name_alias[] = "/tmp/test-files_same.alias";
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
         assert_se(symlink(name, name_alias) >= 0);
@@ -37,6 +41,8 @@ static void test_is_symlink(void) {
         char name_link[] = "/tmp/test-is_symlink.link";
         _cleanup_close_ int fd = -1;
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
         assert_se(symlink(name, name_link) >= 0);
@@ -50,17 +56,33 @@ static void test_is_symlink(void) {
 }
 
 static void test_path_is_fs_type(void) {
+        log_info("/* %s */", __func__);
+
         /* run might not be a mount point in build chroots */
         if (path_is_mount_point("/run", NULL, AT_SYMLINK_FOLLOW) > 0) {
                 assert_se(path_is_fs_type("/run", TMPFS_MAGIC) > 0);
                 assert_se(path_is_fs_type("/run", BTRFS_SUPER_MAGIC) == 0);
         }
-        assert_se(path_is_fs_type("/proc", PROC_SUPER_MAGIC) > 0);
+        if (path_is_mount_point("/proc", NULL, AT_SYMLINK_FOLLOW) > 0) {
-        assert_se(path_is_fs_type("/proc", BTRFS_SUPER_MAGIC) == 0);
+                assert_se(path_is_fs_type("/proc", PROC_SUPER_MAGIC) > 0);
+                assert_se(path_is_fs_type("/proc", BTRFS_SUPER_MAGIC) == 0);
+        }
         assert_se(path_is_fs_type("/i-dont-exist", BTRFS_SUPER_MAGIC) == -ENOENT);
 }
 
 static void test_path_is_temporary_fs(void) {
+        const char *s;
+        int r;
+
+        log_info("/* %s */", __func__);
+
+        FOREACH_STRING(s, "/", "/run", "/sys", "/sys/", "/proc", "/i-dont-exist", "/var", "/var/lib") {
+                r = path_is_temporary_fs(s);
+
+                log_info_errno(r, "path_is_temporary_fs(\"%s\"): %d, %s",
+                               s, r, r < 0 ? errno_to_name(r) : yes_no(r));
+        }
+
         /* run might not be a mount point in build chroots */
         if (path_is_mount_point("/run", NULL, AT_SYMLINK_FOLLOW) > 0)
                 assert_se(path_is_temporary_fs("/run") > 0);
@@ -68,13 +90,42 @@ static void test_path_is_temporary_fs(void) {
         assert_se(path_is_temporary_fs("/i-dont-exist") == -ENOENT);
 }
 
+static void test_path_is_read_only_fs(void) {
+        const char *s;
+        int r;
+
+        log_info("/* %s */", __func__);
+
+        FOREACH_STRING(s, "/", "/run", "/sys", "/sys/", "/proc", "/i-dont-exist", "/var", "/var/lib") {
+                r = path_is_read_only_fs(s);
+
+                log_info_errno(r, "path_is_read_only_fs(\"%s\"): %d, %s",
+                               s, r, r < 0 ? errno_to_name(r) : yes_no(r));
+        }
+
+        if (path_is_mount_point("/sys", NULL, AT_SYMLINK_FOLLOW) > 0)
+                assert_se(IN_SET(path_is_read_only_fs("/sys"), 0, 1));
+
+        assert_se(path_is_read_only_fs("/proc") == 0);
+        assert_se(path_is_read_only_fs("/i-dont-exist") == -ENOENT);
+}
+
 static void test_fd_is_ns(void) {
         _cleanup_close_ int fd = -1;
+
+        log_info("/* %s */", __func__);
+
         assert_se(fd_is_ns(STDIN_FILENO, CLONE_NEWNET) == 0);
         assert_se(fd_is_ns(STDERR_FILENO, CLONE_NEWNET) == 0);
         assert_se(fd_is_ns(STDOUT_FILENO, CLONE_NEWNET) == 0);
 
-        assert_se((fd = open("/proc/self/ns/mnt", O_CLOEXEC|O_RDONLY)) >= 0);
+        fd = open("/proc/self/ns/mnt", O_CLOEXEC|O_RDONLY);
+        if (fd < 0) {
+                assert_se(errno == ENOENT);
+                log_notice("Path %s not found, skipping test", "/proc/self/ns/mnt");
+                return;
+        }
+        assert_se(fd >= 0);
         assert_se(IN_SET(fd_is_ns(fd, CLONE_NEWNET), 0, -EUCLEAN));
         fd = safe_close(fd);
 
@@ -87,6 +138,8 @@ static void test_fd_is_ns(void) {
 }
 
 static void test_device_major_minor_valid(void) {
+        log_info("/* %s */", __func__);
+
         /* on glibc dev_t is 64bit, even though in the kernel it is only 32bit */
         assert_cc(sizeof(dev_t) == sizeof(uint64_t));
 
@@ -128,11 +181,21 @@ static void test_device_path_make_canonical_one(const char *path) {
         mode_t mode;
         int r;
 
-        assert_se(stat(path, &st) >= 0);
+        log_debug("> %s", path);
-        r = device_path_make_canonical(st.st_mode, st.st_rdev, &resolved);
+
-        if (r == -ENOENT) /* maybe /dev/char/x:y and /dev/block/x:y are missing in this test environment, because we
+        if (stat(path, &st) < 0) {
-                           * run in a container or so? */
+                assert(errno == ENOENT);
+                log_notice("Path %s not found, skipping test", path);
                 return;
+        }
+
+        r = device_path_make_canonical(st.st_mode, st.st_rdev, &resolved);
+        if (r == -ENOENT) {
+                /* maybe /dev/char/x:y and /dev/block/x:y are missing in this test environment, because we
+                 * run in a container or so? */
+                log_notice("Device %s cannot be resolved, skipping test", path);
+                return;
+        }
 
         assert_se(r >= 0);
         assert_se(path_equal(path, resolved));
@@ -145,6 +208,7 @@ static void test_device_path_make_canonical_one(const char *path) {
 }
 
 static void test_device_path_make_canonical(void) {
+        log_info("/* %s */", __func__);
 
         test_device_path_make_canonical_one("/dev/null");
         test_device_path_make_canonical_one("/dev/zero");
@@ -160,10 +224,14 @@ static void test_device_path_make_canonical(void) {
 }
 
 int main(int argc, char *argv[]) {
+        log_show_color(true);
+        test_setup_logging(LOG_INFO);
+
         test_files_same();
         test_is_symlink();
         test_path_is_fs_type();
         test_path_is_temporary_fs();
+        test_path_is_read_only_fs();
         test_fd_is_ns();
         test_device_major_minor_valid();
         test_device_path_make_canonical();