Bug 11159 - Missing invocations of x_unlink() when there was an error within get_file_from_cache().
Missing invocations of x_unlink() when there was an error within get_file_fr...
Status: CLOSED FIXED
Product: ccache
Classification: Unclassified
Component: ccache
dev
All All
: P5 normal
: 3.2.2
Assigned To: Joel Rosdahl
Joel Rosdahl
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-11 16:07 UTC by ishikawa
Modified: 2015-05-10 12:33 UTC (History)
0 users

See Also:


Attachments
Leave comments about missing invocations of x_unlinks when there is an error in get_file_from_cache() failed. (2.05 KB, patch)
2015-03-11 16:07 UTC, ishikawa
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description ishikawa 2015-03-11 16:07:37 UTC
Created attachment 10860 [details]
Leave comments about missing invocations of x_unlinks when there is an error in get_file_from_cache() failed.

CAVEAT: The uploaded patch was created during the work to support
-gsplit-dwarf (FissionDwarf) of GCC.  (See bug 10005 )
Some of the patches have interdependence and if earlier patches are
not applied, the later ones may fail. I will re-create patches
appropriately depending on how an earlier patch is incorporated (or
dropped).

The whole list of broken patches below were uploaded in bug 10005 for
convenience, but now separate bugzilla is filed to follow up on each of it
(except for the main -gsplit-dwarf support, in bug 10005).

 0001-Harden-logging-perror-replaced-with-cc_log-and-fatal.patch
 0002-hash-file-creation-failure-is-recorded-now.patch
 0003-Use-x_unlink-instead-of-unlink-per-HACKING.txt.patch
 0004-make-sure-the-result-of-stat-call-is-checked.patch
 0005-Enhance-log-message-add-prefix-var.-name-and-a-few-a.patch
*0006-warning-about-missing-x_unlinks-when-get_file_from_c.patch
 0007-Support-for-gsplit-dwarf.patch
 0008-Missing-copy-of-diagnostics-file-in-case-of-failure.patch

What this entry is about:

Short explanation.

6: 0006-warning-about-missing-x_unlinks-when-get_file_from_c.patch

   This patch is about adding comments that warn about missing
   x_unlinks when get_file_from_cache() failed. 

   I noticed that during the development of -gsplit-dwarf support that
   somehow unlinking of certain files using x_unlink when
   get_file_from_cache() failed are gone in new code circa the
   beginning of Jan 2014: these unlinking calls were in the summer of
   2014, I think.

   Before, get_file_from_cache() returned error code, and when failure
   occurs, unlinking of files were done.

   I wonder if this removal of unlinking behavior is quite
   intentional, and I just wanted to remind developers these are now
   gone by inserting comments.

   This is actually not related to -gsplit-dwarf and so I am filing
   this separate bugzilla.

   Since this changes ccache.c, it may interfere with the later
   patch application if this is not applied to ccache repository.

   BTW, the changes for pure support of -gsplit-dwarf also have
   similar comments. (see bug 10005)

TIA
Comment 1 Joel Rosdahl 2015-03-16 21:33:50 UTC
If there is an error getting the file from cache (either because the file was missing or because there was a read error), failed() is called by get_file_from_cache. failed() in turn will invoke the real compiler, which will overwrite any output_* files that ccache may already have written, so I don't think there's a need for ccache to remove them. For the cached_* files, I agree that it wouldn't hurt to remove them to keep the cache consistent for the result. (It's not very important to do it, though, since the user using ccache will never see a partial result since failed() is called if any file is missing.)

I committed this in de701185bf2928d70ce732ab03f04a4a031caaa9. Thanks for the suggestion!
Comment 2 Joel Rosdahl 2015-05-10 12:33:44 UTC
Included in v3.2.2.