Implement more sanity tests for Memcheck/Valgrind

Implement more sanity tests for Memcheck/Valgrind

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

[Alexander Potapenko, 2010-03-25 15:19:50]

http://codereview.chromium.org/1242008/diff/4001/5003
File base/tools_sanity_unittest.cc (right):

http://codereview.chromium.org/1242008/diff/4001/5003#newcode70
base/tools_sanity_unittest.cc:70: delete [] foo;
That's "delete[]"

[Timur Iskhodzhanov, 2010-03-25 15:24:26]

I'll disregard this (discussed offline)
On 2010/03/25 15:19:50, Alexander Potapenko wrote:
> http://codereview.chromium.org/1242008/diff/4001/5003
> File base/tools_sanity_unittest.cc (right):
> 
> http://codereview.chromium.org/1242008/diff/4001/5003#newcode70
> base/tools_sanity_unittest.cc:70: delete [] foo;
> That's "delete[]"

[Timur Iskhodzhanov, 2010-03-25 15:26:44]

Hi Lei, Stuart,

I'd like you to review this CL.
It adds more positive Memcheck tests so we know if/when Memcheck is broken.
Also, this may be useful for testing sanity of some similar memory checking
tools (e.g. DrMemory)

Thank you!
Timur

[Lei Zhang, 2010-03-25 17:50:24]

LGTM with nits.

http://codereview.chromium.org/1242008/diff/10001/11003
File base/tools_sanity_unittest.cc (right):

http://codereview.chromium.org/1242008/diff/10001/11003#newcode68
base/tools_sanity_unittest.cc:68: char *foo = new char[10];
style nit: char* foo. There's a bunch of these in the file.

http://codereview.chromium.org/1242008/diff/10001/11003#newcode91
base/tools_sanity_unittest.cc:91: if (!RunningOnValgrind()) return;
style nit: be consistent with the same code above.

[Timur Iskhodzhanov, 2010-03-26 09:57:20]

I'll see if it passes trybots and commit if it's OK

http://codereview.chromium.org/1242008/diff/10001/11003
File base/tools_sanity_unittest.cc (right):

http://codereview.chromium.org/1242008/diff/10001/11003#newcode68
base/tools_sanity_unittest.cc:68: char *foo = new char[10];
According to
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...,
both variants are fine.

also, "type *name" has already being used a few times in this file.

On 2010/03/25 17:50:24, Lei Zhang wrote:
> style nit: char* foo. There's a bunch of these in the file.

http://codereview.chromium.org/1242008/diff/10001/11003#newcode87
base/tools_sanity_unittest.cc:87: LOG(INFO) << "Reading uninitialized int: " <<
foo[5];
I've removed this particular test because it doesn't work on 32-bit Linux for
some reason. Will resolve later.

http://codereview.chromium.org/1242008/diff/10001/11003#newcode91
base/tools_sanity_unittest.cc:91: if (!RunningOnValgrind()) return;
Yeah, missed these nits; Fixed.

On 2010/03/25 17:50:24, Lei Zhang wrote:
> style nit: be consistent with the same code above.

[Alexander Potapenko, 2010-03-26 10:08:27]

http://codereview.chromium.org/1242008/diff/10001/11003
File base/tools_sanity_unittest.cc (right):

http://codereview.chromium.org/1242008/diff/10001/11003#newcode68
base/tools_sanity_unittest.cc:68: char *foo = new char[10];
I second Timur's point.

Timur, could you please make the rest of the file consistent, i.e. fix the
"bool* value_" in line 27?

[Timur Iskhodzhanov, 2010-03-26 10:55:18]

http://codereview.chromium.org/1242008/diff/10001/11003
File base/tools_sanity_unittest.cc (right):

http://codereview.chromium.org/1242008/diff/10001/11003#newcode68
base/tools_sanity_unittest.cc:68: char *foo = new char[10];
On 2010/03/26 10:08:27, Alexander Potapenko wrote:
> I second Timur's point.
> 
> Timur, could you please make the rest of the file consistent, i.e. fix the
> "bool* value_" in line 27?
> 

Done.

[Lei Zhang, 2010-03-26 22:34:31]

On 2010/03/26 09:57:20, Timur Iskhodzhanov wrote:
> According to
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...,
> both variants are fine.
> 
> also, "type *name" has already being used a few times in this file.

http://dev.chromium.org/developers/coding-style superseds the Google style
guide, and it specifically says:

"Foo* bar and const Foo& bar, not Foo *bar and const Foo &bar."

It's already checked in, oh well. At least the file is internally consistent
now.