Created attachment 10858 [details] Make sure to check the result of stat() call always. 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. 4: 0004-make-sure-the-result-of-stat-call-is-checked.patch This patch makes sure that the result of stat() call is always checked and error action is taken if stat() does not return 0. 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. (see bug 10005) TIA
> + /* > + * Don't forget to check the return value of stat and act accordingly. > + */ I don't think that this comment tells the reader of the code anything more than the actual code does, so I suggest removing it. > + if (stat(manifest_path, &st) == 0) { Indent with tabs, not spaces. (Yes, I'm considering to reindent the whole code base with spaces instead, but until then... :-)) > + cc_log("stat on manifest (manifest_path) %s failed (%s), so size statistics is incorrect.", Perhaps reformulate as "Failed to stat %s: %s"? > + cc_log("stat on cached_stderr=%s failed (%s)", Same here?
(In reply to Joel Rosdahl from comment #1) Thank you for your comment. I will create a new patch based on your suggestion. I have been using mercurium (hg) [mozilla code base dicated the use of hg for quite some time], but now I need to learn how to use git for re-creating new patch, etc. Oh well... TIA
Created attachment 10882 [details] Make sure to check the result of stat() call always (take 2) Here is the new patch. I changed the messages according to your suggestions and removed a comment, etc. Yes, I carefully replaced whitespaces to tabs. This is very awkward, indeed. TIA
Applied (with minor changes), thanks! > This is very awkward, indeed. "Smart tabs" work perfecly for me, so it's a bit strange that it doesn't work for you. Another thing that I find invaluable (regardless if code is indented with spaces, tabs or "smart tabs") is to configure the editor to show difference between spaces and tabs.
Included in v3.2.2.