Code review: Strengthen range checks in minidump.cc (#173)

[Mark Mentovai]

Brian-

I've got a patch to strengthen some of the overflow and range checks
in minidump.cc, it should be a quick review.

http://code.google.com/p/google-breakpad/issues/detail?id=173
http://code.google.com/p/google-breakpad/issues/attachment?aid=-4790929031470975587&name=breakpad.173.1.patch

Mark

[Mark Mentovai]

numeric_limits<> may be better than the stuff from <stdint.h>.

[Mark Mentovai]

A newer version of the patch that uses numeric_limits<>:

http://code.google.com/p/google-breakpad/issues/attachment?aid=4488819470907578410&name=breakpad.173.2.patch

[Mark Mentovai]

I could have used typeof with this, but most of the names are too long
that it seemed like it would impact readability negatively.

Mark

[Brian Ryner]

I wonder if it would be more readable to add static consts for the commonly-used limits, e.g.

static const u_int64_t kuint64max = numeric_limits<u_int64_t>::max();

Looks good otherwise.

--
-Brian

[Mark Mentovai]

I like that, but I don't think we should do it. Although
numeric_limits<u_int64_t>::max() doesn't really involve a function
call in the strict sense, that's sort of dependent on the
implementation in <limits> AND on the compiler doing inlining. Plus,
it LOOKS like a function call. We shouldn't initialize a global
variable with a function call.

Mark

[Brian Ryner]

I guess if the call wasn't inlined, there could be initialization ordering problems with the c++ standard library?

I almost wonder if it would be better to just use our own constants, in that case.  It's not like we ever use numeric_limits<> with a variable template parameter or anything like that.  I don't really care that much though if you feel that numeric_limits is the right solution.

Thanks,

--
-Brian

[Mark Mentovai]

I prefer numeric_limits<> and other system-provided values because
these are really properties of the machine. Not like we care about
any oddball machines that don't use two's complement representation,
but it's good practice.

Mark