[cron] fixes

Author: jgarzik

Cargo.toml

@@ -34,7 +34,6 @@ members = [
 	"uucp",
 	"xform",
 	"i18n",
-	"cron",
 ]
 
 [workspace.package]

cron/at.rs

@@ -1347,7 +1347,7 @@ mod timespec {
         TimeDateIncrement {
             time: Time,
             date: Date,
-            inrement: Increment,
+            increment: Increment,
         },
         Nowspec(Nowspec),
     }
@@ -1397,12 +1397,12 @@ mod timespec {
             let date = Date::from_str(&s[time_index..date_index])?;
 
             if date_index != string_length {
-                let inrement = Increment::from_str(&s[date_index..])?;
+                let increment = Increment::from_str(&s[date_index..])?;
 
                 return Ok(Self::TimeDateIncrement {
                     time,
                     date,
-                    inrement,
+                    increment,
                 });
             }
 
@@ -1439,15 +1439,15 @@ mod timespec {
                 Timespec::TimeDateIncrement {
                     time,
                     date,
-                    inrement,
+                    increment,
                 } => {
                     let time = time.to_naive_time()?;
                     let date = date.to_naive_date()?;
 
                     let date_time = NaiveDateTime::new(date, time)
                         .and_utc()
                         .checked_add_signed(
-                            chrono::TimeDelta::from_std(inrement.to_duration()).ok()?,
+                            chrono::TimeDelta::from_std(increment.to_duration()).ok()?,
                         )?;
 
                     match date_time < Utc::now() {
@@ -1889,7 +1889,7 @@ mod timespec {
                         day_number: DayNumber(NonZero::new(4).expect("valid")),
                         year_number: YearNumber(2024)
                     },
-                    inrement: Increment::Plus {
+                    increment: Increment::Plus {
                         number: 1,
                         period: IncPeriod::Day
                     }
@@ -1933,7 +1933,7 @@ mod timespec {
                     day_number: DayNumber(NonZero::new(4).expect("valid")),
                     year_number: YearNumber(3000),
                 },
-                inrement: Increment::Plus {
+                increment: Increment::Plus {
                     number: 1,
                     period: IncPeriod::Day,
                 },

cron/crond.rs

@@ -109,6 +109,10 @@ fn acquire_lock() -> Result<File, CronError> {
         .open(PID_FILE)
         .map_err(CronError::PidFile)?;
 
+    // SAFETY: flock() is safe to call here because:
+    // 1. file.as_raw_fd() returns a valid file descriptor from an open File
+    // 2. LOCK_EX | LOCK_NB are valid flags for exclusive non-blocking lock
+    // 3. We check the return value and handle errors appropriately
     unsafe {
         if libc::flock(file.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) != 0 {
             return Err(CronError::AlreadyRunning);
@@ -124,6 +128,12 @@ fn acquire_lock() -> Result<File, CronError> {
 
 /// Create new daemon process of crond
 fn setup() -> i32 {
+    // SAFETY: These libc calls implement the standard Unix daemon pattern:
+    // 1. fork() creates child process; parent exits, child continues
+    // 2. setsid() creates new session, detaches from controlling terminal
+    // 3. chdir("/") prevents holding directory mounts open
+    // 4. close(STD*_FILENO) closes inherited file descriptors
+    // All calls have defined behavior and we check fork() return value.
     unsafe {
         use libc::*;
 
@@ -153,10 +163,11 @@ extern "C" fn handle_sighup(_: libc::c_int) {
 
 /// Handles SIGCHLD signal to reap zombie child processes
 extern "C" fn handle_sigchld(_: libc::c_int) {
-    unsafe {
-        // Reap all zombie children without blocking
-        while libc::waitpid(-1, std::ptr::null_mut(), libc::WNOHANG) > 0 {}
-    }
+    // SAFETY: waitpid() is async-signal-safe per POSIX.
+    // -1 means wait for any child, WNOHANG returns immediately if no zombie.
+    // null status pointer is valid when we don't need exit status.
+    // Loop reaps all available zombies without blocking.
+    unsafe { while libc::waitpid(-1, std::ptr::null_mut(), libc::WNOHANG) > 0 {} }
 }
 
 /// Daemon loop
@@ -203,6 +214,9 @@ fn main() -> Result<(), Box<dyn Error>> {
     // Acquire PID file lock (keep handle alive for the daemon's lifetime)
     let _pid_lock = acquire_lock()?;
 
+    // SAFETY: signal() is safe to call with valid signal numbers and
+    // extern "C" function handlers. SIGHUP and SIGCHLD are standard signals.
+    // The handlers are async-signal-safe (only call async-signal-safe functions).
     unsafe {
         libc::signal(libc::SIGHUP, handle_sighup as usize);
         libc::signal(libc::SIGCHLD, handle_sigchld as usize);
@@ -212,5 +226,7 @@ fn main() -> Result<(), Box<dyn Error>> {
 }
 
 fn sleep(target: u32) {
+    // SAFETY: libc::sleep() is safe with any u32 value; it simply
+    // suspends execution for the specified number of seconds.
     unsafe { libc::sleep(target) };
 }

cron/job.rs

@@ -294,6 +294,11 @@ impl CronJob {
     }
 
     pub fn run_job(&self) -> std::io::Result<()> {
+        // SAFETY: fork() is safe to call here because:
+        // 1. We immediately check the return value for errors (pid < 0)
+        // 2. The child process (pid == 0) immediately exec()s a new process
+        // 3. The parent process returns immediately without shared state issues
+        // 4. We use Command::exec() which replaces the child process entirely
         unsafe {
             let pid = libc::fork();
             if pid < 0 {

cron/tests/at/mod.rs

@@ -136,7 +136,7 @@ fn test4() {
 }
 
 #[test]
-fn test6() {
+fn test5() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -159,7 +159,7 @@ fn test6() {
 }
 
 #[test]
-fn test7() {
+fn test6() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -182,7 +182,7 @@ fn test7() {
 }
 
 #[test]
-fn test8() {
+fn test7() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -205,7 +205,7 @@ fn test8() {
 }
 
 #[test]
-fn test9() {
+fn test8() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -228,7 +228,7 @@ fn test9() {
 }
 
 #[test]
-fn test10() {
+fn test9() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -250,7 +250,7 @@ fn test10() {
 }
 
 #[test]
-fn test11() {
+fn test10() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -279,7 +279,7 @@ fn test11() {
 }
 
 #[test]
-fn test12() {
+fn test11() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");
@@ -309,7 +309,7 @@ fn test12() {
 }
 
 #[test]
-fn test13() {
+fn test12() {
     let _lock = TEST_MUTEX.lock().unwrap();
     let (_temp_dir, dir_path) = setup_test_env();
     fs::create_dir(&dir_path).expect("Unable to create test directory");