You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
back_finalize() protects an --update re-fetch by moving the existing file aside before truncating it (the #77 follow-up, src/htsback.c around line 3155):
if (fexist_utf8(back[i].url_sav)) {
if (create_back_tmpfile(opt, &back[i], "bak") !=0||RENAME(back[i].url_sav, back[i].tmpfile) !=0) {
back[i].tmpfile=NULL;
}
}
That RENAME is bare, and on Windows RENAME refuses an existing target. <file>.bak outlives a run that was killed mid-transfer, because back_finalize_backup() removes it only on commit or after a successful restore. From then on every re-fetch of that URL takes the tmpfile = NULL branch, filecreate() truncates the live copy, and there is no backup to put back when the transfer aborts. That is the protection #77 added, off.
Nothing is logged on that branch, so the crawl looks normal while the guard is silently disabled. POSIX is unaffected: rename(2) clobbers.
hts_rename_over() (src/htstools.c, added in #754) is the obvious fix, but dropping it in is a behaviour change rather than a refactor. It would clobber the stale .bak instead of failing, so it deserves its own change and a test that leaves a .bak behind before the update pass.
Two related spots worth deciding on at the same time:
hts_rename() at src/htscache.c:860 is one more wrapper around RENAME, still returning the rename(2) int convention that Three hand-rolled copies of the unlink-then-rename fallback, with two different return conventions #726 set out to remove. Its only caller (line 1007) hand-rolls the clobber by unlinking hts-cache/old.zip first. It can adopt the helper, or keep its separate diagnostics and say why in a comment.
back_delayed_rename() (src/htsback.c:1517) is deliberately non-clobbering and should stay that way; its comment already says so.
back_finalize()protects an--updatere-fetch by moving the existing file aside before truncating it (the #77 follow-up,src/htsback.caround line 3155):That
RENAMEis bare, and on WindowsRENAMErefuses an existing target.<file>.bakoutlives a run that was killed mid-transfer, becauseback_finalize_backup()removes it only on commit or after a successful restore. From then on every re-fetch of that URL takes thetmpfile = NULLbranch,filecreate()truncates the live copy, and there is no backup to put back when the transfer aborts. That is the protection #77 added, off.Nothing is logged on that branch, so the crawl looks normal while the guard is silently disabled. POSIX is unaffected:
rename(2)clobbers.hts_rename_over()(src/htstools.c, added in #754) is the obvious fix, but dropping it in is a behaviour change rather than a refactor. It would clobber the stale.bakinstead of failing, so it deserves its own change and a test that leaves a.bakbehind before the update pass.Two related spots worth deciding on at the same time:
hts_rename()atsrc/htscache.c:860is one more wrapper aroundRENAME, still returning therename(2)int convention that Three hand-rolled copies of the unlink-then-rename fallback, with two different return conventions #726 set out to remove. Its only caller (line 1007) hand-rolls the clobber by unlinkinghts-cache/old.zipfirst. It can adopt the helper, or keep its separate diagnostics and say why in a comment.back_delayed_rename()(src/htsback.c:1517) is deliberately non-clobbering and should stay that way; its comment already says so.