Infrastructure · rsync

Fixed a 21-Year-Old rsync Bug

June 20265 min read

delete.c  |  RsyncProject/rsync#967

/* before: S_ISDIR routed directly to do_rmdir_at() */
  do_rmdir_at(dfd, fname);

/* after: rmdir-first as atomic emptiness proof */
+ ret = do_rmdir_at(dfd, fname);
+ if (ret == 0 && backup_dir)
+   make_bak_dir(backup_dir_buf);
+ /* Solaris: EEXIST == ENOTEMPTY (both POSIX-valid) */
+ if (errno == EEXIST || errno == ENOTEMPTY)
+   return DR_NOT_EMPTY;

Background

rsync --backup --backup-dir --delete is a common incremental backup pattern. The expected behavior is that deleted items — both files and directories — are moved to --backup-dir. Files worked. Empty directories silently vanished.

Root Cause

delete_item() in delete.c had a branch for S_ISDIR that called do_rmdir_at() directly and returned — it never reached the backup-dir logic. The backup path only existed for files. This was present in the codebase for 21 years undetected because the silent drop only manifested when a directory was empty at sync time.

The Fix

Use do_rmdir_at() as an atomic emptiness probe — if it returns 0, the directory was empty and we can safely call make_bak_dir() to recreate it under --backup-dir. If it returns non-zero, the directory wasn't empty, which means files inside already triggered their own backup paths. Added Solaris compat: on Solaris, EEXIST == ENOTEMPTY, so both errno values return DR_NOT_EMPTY.

Evidence

Reproduced locally with a test sync tree. Verified backup-dir received the directory after patch. Confirmed Solaris EEXIST behaviour from POSIX documentation and rsync's own existing errno handling patterns.

Result

PR #967 submitted upstream to WayneD/rsync. Backward-compatible — adds new behavior only when backup_dir is set, existing behavior unchanged.