Bug 11157 - Always check the result of stat() system call
Always check the result of stat() system call
Product: ccache
Classification: Unclassified
Component: ccache
All All
: P5 normal
: 3.2.2
Assigned To: Joel Rosdahl
Joel Rosdahl
Depends on:
  Show dependency treegraph
Reported: 2015-03-11 15:57 UTC by ishikawa
Modified: 2015-10-31 14:34 UTC (History)
2 users (show)

See Also:

Make sure to check the result of stat() call always. (2.02 KB, patch)
2015-03-11 15:57 UTC, ishikawa
no flags Details
Make sure to check the result of stat() call always (take 2) (1.59 KB, patch)
2015-03-18 16:01 UTC, ishikawa
ishikawa: review? (joel)

Note You need to log in before you can comment on or make changes to this bug.
Description ishikawa 2015-03-11 15:57:08 UTC
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

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).


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)

Comment 1 Joel Rosdahl 2015-03-14 15:23:31 UTC
> +                /*
> +                 * 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?
Comment 2 ishikawa 2015-03-17 06:33:51 UTC
(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...

Comment 3 ishikawa 2015-03-18 16:01:07 UTC
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.

Comment 4 Joel Rosdahl 2015-03-18 19:13:09 UTC
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.
Comment 5 Joel Rosdahl 2015-05-10 12:33:26 UTC
Included in v3.2.2.