fix(p7-times): dir-time entries only record (never create dirs); bound/chunk dir-time frames; harden list add; docs+tests
Review fixes for Phase 7 Wave D. #1 (HIGH): STATUS_DIR_TIMES entries no longer create directories. A new receiver-only File.dir_time_only flag marks dir-time entries; file_save_to_disk_full short-circuits them as FILE_SAVE_SKIPPED before any device/dir branch, so the sink still accumulates metadata into the deferred DirTimeList but creates nothing. Empty source dirs stay untransferred (-a), -m/--prune-empty-dirs semantics are preserved, and a pre-existing regular file/symlink at an empty-dir mirror path no longer aborts the transfer. dir_time_list_apply fstatat()s the leaf (AT_SYMLINK_NOFOLLOW) and skips absent/non-directory paths QUIETLY; only a real existing directory is stamped. Also initialize File.dir_time_only in file_create() (uninitialised garbage otherwise). #2 (MED): send_dir_times() chunks entries into repeated STATUS_DIR_TIMES frames of at most MAX_MANIFEST_ENTRIES, matching the receiver's per-frame bound; the tautological > INT_MAX check is gone. #3 (LOW): dir_time_list_add() assigns each grown array right after its realloc (no dangling) and advances capacity only after both succeed. #4 (LOW): RSYNC_COMPAT.md -- STATUS_MKDIR carries metadata, dir times are transmitted via STATUS_DIR_TIMES and applied at the end, empty dirs are still never created; -m rationale, -O row and Wave D notes updated. Summary counts untouched. #5 (LOW): integration tests for the three #1 scenarios (empty-dir non-creation under -a and -a -m, collision non-abort), scanner test now covers empty-dir capture, and test_file_restore_symlink_metadata asserts the positive apply path when supported. PROTOCOL_VERSION stays 2.17.0; config-frame layout unchanged.
This commit is contained in:
+4
-3
@@ -551,9 +551,10 @@ typedef struct Config {
|
||||
* -J/--omit-link-times REAL by adding directory and symlink time preservation.
|
||||
* The config-frame LAYOUT is unchanged (the omit flags already crossed the
|
||||
* wire), but the FRAME STREAM gains a new terminal frame: after all file data
|
||||
* and the optional delete manifest, the sender transmits one STATUS_DIR_TIMES
|
||||
* frame (a count followed by (path, metadata) pairs) carrying every source
|
||||
* directory's captured times, so the receiver can apply them AFTER all of a
|
||||
* and the optional delete manifest, the sender transmits STATUS_DIR_TIMES
|
||||
* frame(s) (each a count followed by (path, metadata) pairs, chunked so no
|
||||
* frame exceeds the receiver's MAX_MANIFEST_ENTRIES bound) carrying every
|
||||
* source directory's captured times, so the receiver can apply them AFTER all of a
|
||||
* directory's children have been written (writing a child bumps the parent's
|
||||
* mtime). Symlink entries already carry their metadata on the STATUS_SYMLINK
|
||||
* frame; the receiver now applies it (utimensat/lchown with
|
||||
|
||||
@@ -111,6 +111,7 @@ File* file_create(const char* path) {
|
||||
file->metadata = NULL;
|
||||
file->skip = false;
|
||||
file->is_dir = false;
|
||||
file->dir_time_only = false;
|
||||
file->basis_link = NULL;
|
||||
file->link_group = 0;
|
||||
file->link_first = false;
|
||||
|
||||
@@ -551,6 +551,18 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
return FILE_SAVE_ERROR;
|
||||
}
|
||||
|
||||
/* P7 Wave D #1: a STATUS_DIR_TIMES entry is RECORD-ONLY. The scanner
|
||||
captures every traversed directory -- including empty ones whose parents
|
||||
were never created by a child write and directories pruned by
|
||||
-m/--prune-empty-dirs. Creating them here would resurrect empty
|
||||
directories (an -a behavior change) and could abort the whole transfer on a
|
||||
pre-existing regular file/symlink at the mirror path. Short-circuit before
|
||||
any device/write-devices/directory branch and report it as skipped so the
|
||||
sink still accumulates its metadata for the deferred DirTimeList
|
||||
application, but create nothing. */
|
||||
if (file->dir_time_only)
|
||||
return FILE_SAVE_SKIPPED;
|
||||
|
||||
/* Device/special node (--devices/--specials): recreate the node instead of
|
||||
writing content (privilege-gated, confined, rdev-validated). */
|
||||
if (file->is_special)
|
||||
@@ -2174,6 +2186,12 @@ bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetad
|
||||
size_t new_capacity = list->capacity == 0 ? 16 : list->capacity * 2;
|
||||
if (new_capacity < list->capacity)
|
||||
return false;
|
||||
/* Assign each grown array as soon as its realloc succeeds: the old block is
|
||||
already freed by then, so discarding the pointer would dangle. capacity
|
||||
is advanced only after BOTH reallocs succeed, so a partial failure leaves
|
||||
capacity no larger than the entries allocation (the paths array may be
|
||||
over-allocated, which is harmless) -- never a mismatched list the next
|
||||
add could write past. */
|
||||
char** grown_paths = realloc(list->paths, new_capacity * sizeof(char*));
|
||||
if (!grown_paths)
|
||||
return false;
|
||||
@@ -2202,14 +2220,26 @@ void dir_time_list_apply(const DirTimeList* list, const char* root_directory) {
|
||||
continue;
|
||||
char* leaf = NULL;
|
||||
/* The parent walk is fd-relative and O_NOFOLLOW, so a symlink planted in a
|
||||
parent component can never redirect the utimensat outside the root. The
|
||||
final component is a directory; AT_SYMLINK_NOFOLLOW additionally refuses
|
||||
to follow a same-named symlink (a --keep-dirlinks style path). */
|
||||
parent component can never redirect the utimensat outside the root. */
|
||||
int parent_fd = file_open_secure_parent(dir_path, &leaf, false);
|
||||
if (parent_fd < 0) {
|
||||
free(dir_path);
|
||||
continue;
|
||||
}
|
||||
/* A dir-time entry only records metadata: the directory is (deliberately)
|
||||
not created from it, so an empty source directory (or one pruned by
|
||||
-m/--prune-empty-dirs) may well not exist here. Skip absent paths
|
||||
QUIETLY rather than warning for every one, and apply the times only to a
|
||||
real directory that does exist. AT_SYMLINK_NOFOLLOW keeps a same-named
|
||||
symlink from being followed; a pre-existing regular file/symlink is not a
|
||||
directory, so it is left completely untouched. */
|
||||
struct stat st;
|
||||
if (fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) != 0 || !S_ISDIR(st.st_mode)) {
|
||||
close(parent_fd);
|
||||
free(leaf);
|
||||
free(dir_path);
|
||||
continue;
|
||||
}
|
||||
struct timespec times[2] = {
|
||||
{.tv_sec = 0, .tv_nsec = UTIME_OMIT},
|
||||
{.tv_sec = list->entries[i].mtime_sec, .tv_nsec = list->entries[i].mtime_nsec}};
|
||||
@@ -2265,11 +2295,13 @@ File* file_receive_directory(int file_descriptor, const Config* config) {
|
||||
return file;
|
||||
}
|
||||
|
||||
/* Receive one directory-time entry from the terminal STATUS_DIR_TIMES frame:
|
||||
* the destination-relative wire path and (when metadata is negotiated) the
|
||||
* directory's metadata frame. The created File is an is_dir entry routed
|
||||
* through the regular store_file sink, exactly like a STATUS_MKDIR entry, so
|
||||
* the same deferred DirTimeList application covers both. */
|
||||
/* Receive one directory-time entry from a STATUS_DIR_TIMES frame: the
|
||||
* destination-relative wire path and (when metadata is negotiated) the
|
||||
* directory's metadata frame. The created File is an is_dir, dir_time_only
|
||||
* entry routed through the regular store_file sink: the sink records its
|
||||
* metadata into the deferred DirTimeList but never creates the directory (the
|
||||
* scanner captures every traversed directory, including empty ones). Unlike a
|
||||
* STATUS_MKDIR entry, this one must not create anything. */
|
||||
File* file_receive_dir_time(int file_descriptor, const Config* config) {
|
||||
char* path = receive_wire_str(file_descriptor);
|
||||
if (path == NULL)
|
||||
@@ -2287,6 +2319,7 @@ File* file_receive_dir_time(int file_descriptor, const Config* config) {
|
||||
if (!file)
|
||||
return NULL;
|
||||
file->is_dir = true;
|
||||
file->dir_time_only = true;
|
||||
if (config && config->use_metadata) {
|
||||
int meta_ok = 1;
|
||||
file->metadata = metadata_receive(file_descriptor, &meta_ok);
|
||||
|
||||
@@ -18,7 +18,7 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped);
|
||||
|
||||
/* P7 Wave D directory-time accumulator. The receiver collects the metadata of
|
||||
* every directory it creates/receives (STATUS_MKDIR with metadata and/or the
|
||||
* terminal STATUS_DIR_TIMES frame) and applies the times only at the END of the
|
||||
* trailing STATUS_DIR_TIMES frame(s)) and applies the times only at the END of the
|
||||
* transfer, after all children have been written and after the delete /
|
||||
* --delay-updates phases have committed (writing or removing a child bumps the
|
||||
* parent's mtime). -O/--omit-dir-times skips the application entirely. The
|
||||
@@ -36,8 +36,10 @@ void dir_time_list_free(DirTimeList* list);
|
||||
* allocation failure (the caller fails the transfer). */
|
||||
bool dir_time_list_add(DirTimeList* list, const char* wire_path, const FileMetadata* metadata);
|
||||
/* Apply every accumulated directory's mtime (and atime when captured) beneath
|
||||
* `root_directory`, confined fd-relative. Best-effort per entry: a missing or
|
||||
* unreachable directory is skipped with a warning, never fatal. */
|
||||
* `root_directory`, confined fd-relative. Best-effort per entry: an absent
|
||||
* directory (an empty/pruned source dir that was deliberately not created) or a
|
||||
* non-directory at the path is skipped QUIETLY, an unreachable one with a
|
||||
* warning, and never fatal. */
|
||||
void dir_time_list_apply(const DirTimeList* list, const char* root_directory);
|
||||
|
||||
/* A received delete-manifest frame: the keep-set (`keeps`, destination-relative
|
||||
|
||||
@@ -42,6 +42,14 @@ typedef struct {
|
||||
/* True when this entry is an explicit directory entry (--dirs mode): the
|
||||
* receiver creates the directory instead of writing a regular file. */
|
||||
bool is_dir;
|
||||
/* Receiver-only (P7 Wave D): this is a STATUS_DIR_TIMES entry. It carries a
|
||||
* traversed source directory's metadata for DEFERRED application, but must
|
||||
* NEVER create the directory: the scanner captures every traversed directory
|
||||
* (including empty ones whose parents no child write created), so creation
|
||||
* would resurrect the empty dirs that FastSync deliberately never transfers.
|
||||
* file_save_to_disk_full short-circuits such an entry as FILE_SAVE_SKIPPED,
|
||||
* and the sink still accumulates the metadata into its DirTimeList. */
|
||||
bool dir_time_only;
|
||||
/* Receiver-only, --link-dest: when set, install the destination entry as a
|
||||
* hard link to this absolute (root-confined) path instead of writing
|
||||
* `data`. The matching code has already verified the link target's content
|
||||
|
||||
@@ -69,7 +69,7 @@ typedef struct {
|
||||
bool scan_stopped_early;
|
||||
/* P7 Wave D: captured source directory times, filled by the scanner thread
|
||||
* (and its parallel workers, guarded by dir_entries_mutex) and drained by the
|
||||
* sender thread in the terminal STATUS_DIR_TIMES frame. Owned by the
|
||||
* sender thread in trailing STATUS_DIR_TIMES frame(s). Owned by the
|
||||
* context; NULL for non-metadata transfers. */
|
||||
ArrayList* dir_entries;
|
||||
mtx_t dir_entries_mutex;
|
||||
|
||||
Reference in New Issue
Block a user