Coverity fixes CID=15870,13529 Check pointer before assign, resource leak.

Coverity fixes CID=15870,13529 Check pointer before assign, resource leak.



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90599

[Greg Billock, 2011-06-22 21:52:43]

Ricardo, could you check out this one as well? Thanks.

[rvargas (doing something else), 2011-06-23 03:43:53]

You'll need an owner for base anyway.

http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc
File base/platform_file_posix.cc (right):

http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc#new...
base/platform_file_posix.cc:59: *error_code = *error_code ?
PLATFORM_FILE_ERROR_FAILED :
I have no idea why this code was changed from what it was (r79058)... it looks
like an unintentional change.

This code should not be returning PLATFORM_FILE_OK when it fails on an
unreachable path. If this breaks something, the caller should be fixed.

http://codereview.chromium.org/7222010/diff/1/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

http://codereview.chromium.org/7222010/diff/1/base/shared_memory_posix.cc#new...
base/shared_memory_posix.cc:146: file_util::CloseFile(fp);
nit: it seems ScopedFILE could be used by this method, but that's up to you.

[Greg Billock, 2011-06-23 16:58:32]

Realized Jeremy left. :-( I'll set this back. There's nothing in the change
(including snappy build) to indicate why this was needed.

http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc
File base/platform_file_posix.cc (right):

http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc#new...
base/platform_file_posix.cc:59: *error_code = *error_code ?
PLATFORM_FILE_ERROR_FAILED :
On 2011/06/23 03:43:53, rvargas wrote:
> I have no idea why this code was changed from what it was (r79058)... it looks
> like an unintentional change.
> 
> This code should not be returning PLATFORM_FILE_OK when it fails on an
> unreachable path. If this breaks something, the caller should be fixed.

Yeah, I think the previous version was right. I'll copy jorlow on this change.

[jorlow, 2011-06-23 17:36:23]

On 2011/06/23 16:58:32, Greg Billock wrote:
> Realized Jeremy left. :-( I'll set this back. There's nothing in the change
> (including snappy build) to indicate why this was needed.
> 
> http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc
> File base/platform_file_posix.cc (right):
> 
>
http://codereview.chromium.org/7222010/diff/1/base/platform_file_posix.cc#new...
> base/platform_file_posix.cc:59: *error_code = *error_code ?
> PLATFORM_FILE_ERROR_FAILED :
> On 2011/06/23 03:43:53, rvargas wrote:
> > I have no idea why this code was changed from what it was (r79058)... it
looks
> > like an unintentional change.
> > 
> > This code should not be returning PLATFORM_FILE_OK when it fails on an
> > unreachable path. If this breaks something, the caller should be fixed.
> 
> Yeah, I think the previous version was right. I'll copy jorlow on this change.

I only vaguely remember this, but I believe I thought I was fixing a bug where
the value was not getting set in this code path.  Looking at it now, I can't
tell why though.  Sorry about that.  :-/

[rvargas (doing something else), 2011-06-23 22:56:44]

LGTM

[commit-bot: I haz the power, 2011-06-27 15:55:11]

Presubmit check for 7222010-7001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
base/platform_file_posix.cc,base/shared_memory_posix.cc

Presubmit checks took 1.1s to calculate.

[Greg Billock, 2011-06-27 16:01:02]

Will, could you have a look at this for base/OWNERS?

[willchan no longer on Chromium, 2011-06-27 16:09:43]

On 2011/06/27 16:01:02, Greg Billock wrote:
> Will, could you have a look at this for base/OWNERS?

LGTM

[commit-bot: I haz the power, 2011-06-27 17:24:29]

Change committed as 90599