Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> Certain lines of the marks file might be corrupted (or the objects
> missing due to a garbage collection), but that's no reason to truncate
> the file and essentially destroy the rest of it.
Hmm, so the issue is:
- we use die_nicely() that calls dump_marks() after writing a crash
report as our die_routine().
- when dump_marks() is called, and export_marks_file names an
existing file, it tries to write marks in it. If we let it go
through, we would end up writing a new marks file based on an
incomplete set of marks we have only half-read in the earlier
step, which is bad, as the resulting one is incomplete, and the
original one that this replaced may have been a good one.
Is that what this change addresses?
I am just wondering if a solution to preserve both files is more
desirable.
This change looks a bit over-eager to discard the dump die_nicely()
is trying to create in one scenario, and a bit less careful at the
same time in another scenario.
- Even if we are reading from somewhere, export_marks_file can
point at a completely new file that is different from
import_marks file, in which case, we are not really losing any
information by freshly creating a new marks file, no?
- Even if we did not read from any existing marks file, if we are
given export_marks_file that names an existing file, wouldn't we
want to avoid corrupting it with a dump from this aborted run?
In other words, I understand the intent of "import-marks-file-done",
but I am not sure if that is so special a case.
If the patch were to tell dump_marks() if the caller is die_nicely()
or not, and stop it from overwriting an existing file, whether we
read from any import-marks file, I would agree with the change a lot
more strongly. An obvious extension of that line of thought would
be to export to an alternative marks file, perhaps to
strbuf_addf("%s.crash", exports_marks_file), if exports_marks_file
already exists if we are called while dying.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html