Fix Clang warning regarding null pointer argument. Fix provided by Ian Wilkinson.

Fix Clang warning regarding null pointer argument.

This warning was showing up in the Clang static analyzer in Xcode: "Null pointer
argument in call to memory copy function"

Fix provided by Ian Wilkinson.
Committed: https://code.google.com/p/google-breakpad/source/detail?r=1162

[Ivan Penkov]

.

[Mark Mentovai]

https://breakpad.appspot.com/569002/diff/1/src/common/memory.h
File src/common/memory.h (right):

https://breakpad.appspot.com/569002/diff/1/src/common/memory.h#newcode199
src/common/memory.h:199: return;
But now you’re leaving a_ alone, which maybe isn’t right.

Perhaps you should let the allocator do its job and then guard the call to
memcpy with this condition.

[Ivan Penkov]

Done.

https://breakpad.appspot.com/569002/diff/1/src/common/memory.h
File src/common/memory.h (right):

https://breakpad.appspot.com/569002/diff/1/src/common/memory.h#newcode199
src/common/memory.h:199: return;
On 2013/04/23 21:48:23, Mark Mentovai wrote:
> But now you’re leaving a_ alone, which maybe isn’t right.
> 
> Perhaps you should let the allocator do its job and then guard the call to
> memcpy with this condition.

Yes, I think what Mark is proposing is cleaner.  Done.

Ian, let us know if this works for you!  Does it address the warning issue?

[Mark Mentovai]

LGTM

[iwilkins]

On 2013/04/23 22:46:05, Ivan Penkov wrote:
> Done.
> 
> https://breakpad.appspot.com/569002/diff/1/src/common/memory.h
> File src/common/memory.h (right):
> 
> https://breakpad.appspot.com/569002/diff/1/src/common/memory.h#newcode199
> src/common/memory.h:199: return;
> On 2013/04/23 21:48:23, Mark Mentovai wrote:
> > But now you’re leaving a_ alone, which maybe isn’t right.
> > 
> > Perhaps you should let the allocator do its job and then guard the call to
> > memcpy with this condition.
> 
> Yes, I think what Mark is proposing is cleaner.  Done.
> 
> Ian, let us know if this works for you!  Does it address the warning issue?

Actually, this still causes a problem. Making this change results in this
warning on line 163:

/Users/iwilkins/gsa.local/google3/third_party/breakpad/src/common/memory.h:163:5:
Called C++ object pointer is null

a_ would end up being set to null and then an attempt would be made to
dereference it.

[iwilkins]

On 2013/04/25 03:06:57, iwilkins wrote:
> On 2013/04/23 22:46:05, Ivan Penkov wrote:
> > Done.
> > 
> > https://breakpad.appspot.com/569002/diff/1/src/common/memory.h
> > File src/common/memory.h (right):
> > 
> > https://breakpad.appspot.com/569002/diff/1/src/common/memory.h#newcode199
> > src/common/memory.h:199: return;
> > On 2013/04/23 21:48:23, Mark Mentovai wrote:
> > > But now you’re leaving a_ alone, which maybe isn’t right.
> > > 
> > > Perhaps you should let the allocator do its job and then guard the call to
> > > memcpy with this condition.
> > 
> > Yes, I think what Mark is proposing is cleaner.  Done.
> > 
> > Ian, let us know if this works for you!  Does it address the warning issue?
> 
> Actually, this still causes a problem. Making this change results in this
> warning on line 163:
> 
>
/Users/iwilkins/gsa.local/google3/third_party/breakpad/src/common/memory.h:163:5:
> Called C++ object pointer is null
> 
> a_ would end up being set to null and then an attempt would be made to
> dereference it.

This diff will fix the issue. It prevents push_back from ever calling Realloc()
with 0 (which could happen if wasteful_vector was constructed with size_hint set
to 0, causing allocated_ to be 0.

--- a/google3/third_party/breakpad/src/common/memory.h
+++ b/google3/third_party/breakpad/src/common/memory.h
@@ -159,7 +159,7 @@ class wasteful_vector {
 
   void push_back(const T& new_element) {
     if (used_ == allocated_)
-      Realloc(allocated_ * 2);
+      Realloc(std::max(allocated_ * 2, 1u));
     a_[used_++] = new_element;
   }