RFR: 8353329: Small memory leak when create GrowableArray with initial size 0

[Stefan Karlsson]

On Tue, 1 Apr 2025 00:18:07 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

> Please review this small fix to avoid 1 byte leak when create a GrowableArray with initial size = 0.
> 
> GrowableArray's c-heap allocator does not check size = 0 and os:malloc(0) will malloc at least 1 byte.

Note that GrowableArrayCHeap already has this code:

  static E* allocate(int max, MemTag mem_tag) {
    if (max == 0) {
      return nullptr;
    }

    return (E*)GrowableArrayCHeapAllocator::allocate(max, sizeof(E), mem_tag);
  }


So, maybe we should just add the same check to GrowableArray? Or, alternatively, remove the check above?

I added a style comment bellow:

src/hotspot/share/utilities/growableArray.cpp line 46:

> 44: void* GrowableArrayCHeapAllocator::allocate(int max, int element_size, MemTag mem_tag) {
> 45:   assert(max >= 0, "integer overflow");
> 46:   if (max == 0) return nullptr;

I would prefer if we don't introduce more code that uses this if/return one-liner into this code. I understand that some parts of our code base uses it, but in code that I maintain / review I try to prevent more introduction of this.

I would like to suggest the following, which adds blank lines to clearly separate that this part of the function is separate from the surrounding code. I want it to be prominent that we have a special case here.

Suggestion:


  if (max == 0) {
    return nullptr;
  }

-------------

Changes requested by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24341#pullrequestreview-2732968879
PR Review Comment: https://git.openjdk.org/jdk/pull/24341#discussion_r2023014005