Page MenuHomeFreeBSD

Remove dependency of rc.d/tmp on grep

Authored by se on Jan 17 2021, 12:29 PM.



The committed version of /etc/rc.d/tmp depends on "mountcritremote" due to its use of grep.

This dependency can easily be removed to allow rc.d/tmp to be executed before /usr/bin is mounted.

Use of a here document avoids complications due to the spawned sub-shell if "df /tmp | while ..." was used.
In that case, the sub-shell would return a success code which then needed to be tested.

If a version without here-document was found to be preferential, I'd suggest a shell function that returns the result of "df /tmp | grep ..." but which does use the while loop from this patch instead of grep.

Test Plan

Apply patch and verify that "mount_md" is invoked if there is no line matching "^/dev/md[0-9].* /tmp" in the output of "df /tmp".

Diff Detail

R10 FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

se requested review of this revision.Jan 17 2021, 12:29 PM
se created this revision.

This fixes at least one dependance on /usr, I see another later, mktemp which is /usr/bin/mktemp. I would of marked it above, but a full file context was not uploaded to this review.

This revision is now accepted and ready to land.Jan 17 2021, 2:09 PM

Remove dependency on mktemp, too, since it also resides in /usr/bin, as noted by rgrimes.

This revision now requires review to proceed.Jan 17 2021, 2:44 PM
cy requested changes to this revision.Jan 17 2021, 2:46 PM

Otherwise it looks good. This has the advantage of:

  1. Less dependency on /usr, as rgrimes mentioned on src-committers@.
  2. Less fork()ing.

Another way to do this, might be df /tmp | while read line... To me it looks cleaner.

This revision now requires changes to proceed.Jan 17 2021, 2:46 PM

As written in the comment of the original submission, a version based on "df /tmp | while ..." can be used instead of the here-doc.
But after implementing both, I did consider the here-doc based version to be both easier to understand and more efficient and thus only proposed that version.


My initial attempt actually used "df /tmp | while ...", but I find the here doc version more straight forward.

Here is a version that uses "df /tmp | while ...":

        df /tmp | while read line; do
                case $line in
                /dev/md[0-9]*\ /tmp)
                        return 1;;
        if [ $? = 0 ]; then
            mount_md ${tmpsize} /tmp "${tmpmfs_flags}"
            chmod 01777 /tmp

The while loop is executed in a sub-shell and thus needs to be exited with "return 1" to pass a value to the if condition.

This is easy to understand if you know about the while loop being executed in a sub-shell, I think that the here-doc version does not require such knowledge.

The pipe-based version requires 2 forked sub-processes (the df command and the shell executing the while loop), but the here-doc version only forks the df process and I do prefer it for these reasons.

Stefan, thanks a lot of taking care of my fallout!


To me the here-doc looks more efficient than pipe.

I had tried to create the temporary directory name without dependency on /dev/random (and thus suggested to use "ps | sha256", or in my temporary version "ps lauxww | grep sha256").

But since mktemp uses random(4), the currently committed version did also depend on /dev/random being operational.
Therefore, the version I want to commit uses "dd if=/dev/random" to generate the test directory.
(If sufficient entropy was not available, "ps lauxww" might actually be preferential, but then I'd expect that there must have been reports of mktemp blocking ...)

Agree with glebius.

This revision is now accepted and ready to land.Jan 18 2021, 9:12 PM
This revision was automatically updated to reflect the committed changes.