Skip to content

hts_rename_over() deletes the destination when the source is missing #779

Description

@xroche

hts_rename_over() in src/htstools.c deletes its destination when the source does not exist:

  if (RENAME(csrc, cdst) == 0)
    return HTS_TRUE;
  /* RENAME does not clobber an existing target on Windows. */
  (void) UNLINK(cdst);
  return RENAME(csrc, cdst) == 0 ? HTS_TRUE : HTS_FALSE;

The UNLINK is there to make the Windows clobber work, but it runs for any failed rename, including ENOENT on the source. A caller that asks to move a file it did not manage to write loses the destination and gets HTS_FALSE, which reads as "nothing happened".

Every current caller happens to be safe. htsback.c:3153 clears back->tmpfile when either the temp creation or the backup rename fails, and htssinglefile.c:1112 only renames under its ok flag. But the contract is not written down anywhere, and the name suggests the opposite. It cost a data-loss bug in #777: warc_commit() renamed one temp per segment, warc_rotate() could leave the segment counter naming a file whose open had failed, and the helper then deleted the previous run's good segment.

Either UNLINK only after a rename that failed for a reason other than a missing source, or document the destructive behaviour at the declaration in htstools.h so the next caller knows to check.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions