Page MenuHomeFreeBSD

/etc/periodic/weekly/310.locate reads /etc/locate.rc
ClosedPublic

Authored by wosch on Oct 25 2021, 6:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 2:54 PM
Unknown Object (File)
Mar 22 2024, 10:57 PM
Unknown Object (File)
Mar 22 2024, 10:57 PM
Unknown Object (File)
Mar 22 2024, 10:57 PM
Unknown Object (File)
Mar 8 2024, 11:24 AM
Unknown Object (File)
Jan 16 2024, 3:17 AM
Unknown Object (File)
Jan 6 2024, 9:43 PM
Unknown Object (File)
Jan 6 2024, 9:43 PM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

wosch requested review of this revision.Oct 25 2021, 6:07 PM

This change is technically correct, to adapt it to a changed path of the locate database.

But I do not like the way "rc" is set to an error code if any of the preparing commands fails, but then the update script is invoked, nonetheless ...

I'd rather write this as:

if touch $locdb && chown nobody $locdb && chmod 644 $locdb
then
    cd /
    echo /usr/libexec/locate.updatedb | nice -n 5 su -fm nobody && rc=0 || rc=3
else
    rc=3
fi

chmod 444 $locdb || rc=3;;

Or even shorter with identical behavior:

cd /
touch $locdb && chown nobody $locdb && chmod 644 $locdb && \
    nice -n 5 su -fm nobody /usr/libexec/locate.updatedb && rc=0 || rc=3

chmod 444 $locdb || rc=3;;

And, in fact, I'd rather write the new locate database to a temp file in the target directory to be able to rename it in place to its final name (and to be able to preserve the old contents, if anything goes wrong).
That would require further changes to both this periodic script and to locate.update (to have it receive the temporary FCODES value on the command line - but to have it keep using the default value, else, to preserve current semantics for the case of a direct invocation from some script).
If there is any interest, I'll develop and test the required changes to both these scripts and create a review.

NB: The "nice -n 5" serves no purpose anymore, since the ULE scheduler tends to give even "nice -n 19" processes the same fraction of CPU cycles as "nice -n 0" processes get ...

This revision is now accepted and ready to land.Oct 26 2021, 12:36 PM

I totally agree that 310.locate and locate.updatedb needs some refactoring.

E.g. if you put "set -e" at the beginning of the script, you don't need to check each command for success, the shell script will do this. The result will be a much more readable shell script.

I totally agree that 310.locate and locate.updatedb needs some refactoring.

E.g. if you put "set -e" at the beginning of the script, you don't need to check each command for success, the shell script will do this. The result will be a much more readable shell script.

With "set -e" the script will be aborted on a failed command, but that could lead to the database access mode not being set back to 444, and there is no control over the return code.

A trap on exit could be used to enforce a sane state when the script is aborted due to a failed command, but that adds extra complexity that is not really required.

IMHO, the second code snippet that I have suggested in my previous comment is the simplest and shortest implementation that preserves all current behavior and prevents the execution of the update script if one of the prior commands failed.