calendar: correct the search order for files
ClosedPublic
Actions

Authored by kevans on Oct 18 2023, 3:29 PM.

Details

Reviewers

bapt

Group Reviewers

Klara

Commits

rG0a82cd4f101c: calendar: correct the search order for files

Summary

Include files that don't begin with a '/' are documented to search the
current directory, then /usr/share/calendar. This hasn't been accurate
for years, since e061f95e7b9b ("Rework calendar(1) parser") rewrote a
lot of this.

Stash off the cwd before we do any chdir()ing around and use that to
honor the same order we'll follow for the -f flag. This may result in
an extra lookup that will fail for the initial calendar file, but I
don't think it's worth the complexity to avoid it.

While we're here, fix the documentation to just reference the order
described in FILES so that we only need to keep it up to date in one
place.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 54085
Build 50975: arc lint + arc unit

Event Timeline

kevans created this revision.Oct 18 2023, 3:29 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 18 2023, 3:29 PM

kevans requested review of this revision.Oct 18 2023, 3:29 PM

Harbormaster completed remote builds in B54060: Diff 129014.Oct 18 2023, 3:29 PM

It could also make sense to:

@@ -654,17 +654,15 @@ opencalin(void)
 
        /* open up calendar file */
        cal_file = calendarFile;
-       if ((fpin = fopen(calendarFile, "r")) == NULL) {
-               if (doall) {
-                       if (chdir(calendarHomes[0]) != 0)
-                               return (NULL);
-                       if (stat(calendarNoMail, &sbuf) == 0)
-                               return (NULL);
-                       if ((fpin = fopen(calendarFile, "r")) == NULL)
-                               return (NULL);
-               } else {
-                       fpin = cal_fopen(calendarFile);
-               }
+       if (doall && (fpin = fopen(calendarFile, "r")) == NULL) {
+               if (chdir(calendarHomes[0]) != 0)
+                       return (NULL);
+               if (stat(calendarNoMail, &sbuf) == 0)
+                       return (NULL);
+               if ((fpin = fopen(calendarFile, "r")) == NULL)
+                       return (NULL);
+       } else if (!doall) {
+               fpin = cal_fopen(calendarFile);
        }
        return (fpin);
 }

Just so that all of the !doall logic is consolidated here.

des added a subscriber: des.Oct 19 2023, 11:28 AM

des added inline comments.

usr.bin/calendar/io.c
147	You're silently ignoring all errors from `openat()` here, is that really what we want or should we only ignore `ENOENT`?

Don't ignore errors from openat()

Harbormaster completed remote builds in B54085: Diff 129101.Oct 19 2023, 2:53 PM

kevans added inline comments.Oct 19 2023, 2:53 PM

usr.bin/calendar/io.c
147	I was torn because we ignore all errors from `fopen` below and just fall through to the rest of the search paths, but that also can lead to false positive messages about the calendar-data port... I think we also want to ignore `ENAMETOOLONG` because then the file just can't exist and that's probably not what the user wanted.