C API: Error allocating / deallocating memory for array

Question

I'm in the process of implementing an API for C. The code base itself is purely written in C++ and I only plan to offer said interface for any consumer using C. The interface is defined in a .h file, whereas the implementation itself is written in C++. I've read multiple times that using C++ to implement a C interface is not the best idea, but it works great in my case.

Anyway the header definition looks similar to this:

extern 'C' {
  typedef struct Person {
    const char *name;
    uint32_t age;
    uint32_t post_code;
  } Person;

  typedef struct PersonArray {
    Person *person;
    size_t size;
  } PersonArray;

  PersonArray *create(size_t size);
  void destroy(PersonArray *array);
  int fillArray(PersonArray *array);
  
}

I'd like the consumer to retrieve a handle for PersonArray, which contains an array of Person structure, allocated with the size passed to the create() function.

Since the implementation is in C++ I've tried to allocate the memory the following way:

static inline Person convert(const otherNamespace::Person &o_person) {
  Person p{};
  p.name = o_person.name;
  p.age = o_person.age;
  p.post_code = o_person.post_code;
  return p;
}

PersonArray *create(size_t size) {
  if (size <= 0) {
    return nullptr;
  }

  PersonArray *array = new PersonArray();
  array->size = size;

  array->person = new Person[size]
  return array;
}

void destory(PersonArray *array) {
  delete array;
}

int fillArray(PersonArray *array) {
  if (array == nullptr) {
    return 1;
  }
  auto data = // retrieve std::vector<otherNamespace::Person> via RPC
  for (auto i{0U}; i < array->size; i++) {
    array->person[i] = convert(data.at(i);
  }
  
  return 0;
}

Unfortunately, this approach does not seem to work correctly, because when using a memchecker like valgrind, there are still blocks on the heap that are not correctly deallocated. I suppose the line new Person[size] does not get deallocated.

Any idea how to fix this memory leak? Or is there another design which would be better suited for this specific use case? If possible, I would really like to keep the implementation in C++.

I count two `new` and one `delete`. So why no `delete array->person`? — KamilCuk, Oct 13 '22 at 08:38
Isn't there a way to delete ```PersonArray``` and deallocate the members of the struct at the same time? Do I really have to first ```delete array->person``` and then call ```delete array``` afterwards? @KamilCuk — Xershy, Oct 13 '22 at 08:41
Also, the code `*array->person[i] = // fill with data` looks odd. Are you doing a shallow copy? Please provide a [mcve]. — G.M., Oct 13 '22 at 08:41
`Isn't there a way to delete PersonArray and deallocate the members of the struct at the same time?` No, there is one heap per program, everything happens sequentially. `Do I really have to first delete array->person and then call delete array afterwards?` Yes. (Why is it confusing? You "really did" two `new`, so now you really have to do two `delete`) — KamilCuk, Oct 13 '22 at 08:42
@G.M. added an edit to my initial post which shows how the data is copied. Not sure if that's the best way, always open to better suggestions. Still learning :) — Xershy, Oct 13 '22 at 08:46
@KamilCuk after adding ```delete array->person``` before the line ```delete array``` valgrind now says ```Mismatched free() / delete / delete []``` — Xershy, Oct 13 '22 at 08:48
I still don't see how `*array->person[i]` can possibly compile given the definitions shown. — G.M., Oct 13 '22 at 08:53
I made a mistake while writing the question, it's supposed to be ```array->person[i]``` without the ```*``` @G.M. — Xershy, Oct 13 '22 at 08:57

carce-bo · Answer 1 · 2022-10-13T15:06:18.943

1

You must use delete on person before array, but since it was allocated with new [] you must delete it with delete [].

void destroy(PersonArray *array) {
  if (array) {
      if (array->person) {
          delete [] array->person;
      }
      delete array;
  }
}

edited Oct 13 '22 at 15:06

answered Oct 13 '22 at 08:51

carce-bo

480
2
10

This indeed solved the problem with valgrind, just out of curiosity, is there a 'nicer' way to allocate the memory without having to use a double free? Maybe my design of using a second struct to keep a handle to the array is the mistake? – Xershy Oct 13 '22 at 09:00
1

There is no nicer way, whilst maintaining a C-friendly struct. C structs do not have constructors and destructors. But even if they did, you'd still have to `delete[]` the `person` array in the destructor. – JonGreen Oct 13 '22 at 09:14

C API: Error allocating / deallocating memory for array

1 Answers1