0

I am trying to use take realloc a 2D array but I keep getting a Segmentation Fault and it seems to almost positively be stemming from how I am reallocating my 2D array. The array is initially specified to be a size of 3 x 26. My goal is to simple reallocate the first array (3) and just multiply it by 2 while keeping the other arrays all a size of 26.

//Array creation
 int ** array_creation (){
    int ** arr = (int **) malloc(buffer_size * sizeof(int *));
        if(arr==NULL){
            cout<<"malloc fail"<<endl;
        }
        for(int i=0;i<buffer_size;i++){
            arr[i] = (int *)malloc(26 * sizeof(int));
            if(arr[i]==NULL){
                cout<<"malloc fail"<<endl;
            }
        }
    return arr;
}
//Variables
buffer_size = 3;
buffer_count = 0;
 if(buffer_count >= buffer_size){
        cout<<"Doubling the size of dynamic arrays!"<<endl;
        buffer_size = buffer_size * 2;
        // arr = (int **) realloc(arr, buffer_size * sizeof(int));
}

So once the buffer_size is equal to the buffer_count I want to just double the first array that would start at 3, so make the 2D array go from 3 array of 26 integers to 6 arrays of 26 integers.

Thank you for any help.

ibucksss
  • 1
  • 6
    Why not remove all the manual array memory management and use `std::vector` ? – Richard Critten Dec 09 '22 at 09:19
  • 2
    Apart from using `std::vector` – if you insist on raw arrays then `new[]` and `delete[]` is more C++-like... – Aconcagua Dec 09 '22 at 09:21
  • 1
    You don't really have a 2D array here. All you have is an addressing syntax similar to a 2D array. You actually have a non-contiguous array of arrays. – paddy Dec 09 '22 at 09:23
  • The `realloc` function only reallocates the memory that you pass to the function, which for your case (when you fix [the `sizeof` problem](https://stackoverflow.com/a/74741061/440558)) will enlarge the array of pointers, but it will ***not*** initialize the new elements of that array. You will end up with an array that contains uninitialized pointers that can't be used. – Some programmer dude Dec 09 '22 at 09:23
  • Also remember that if `realloc` fails, it will return a null pointer, but still leave the original memory untouched. If you assign back to the pointer you pass as argument then you will loose that original memory, leading to a *leak*. – Some programmer dude Dec 09 '22 at 09:26
  • 1
    And as a general note, whenever you must or feel the need to use C-style casting in your C++ program, you should take that as a sign that you're doing something wrong. Also, `NULL` is a C backward compatible macro, which shouldn't really be used in C++ any more. Use `nullptr` for null pointers. Lastly, please invest in [some good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) to learn C++ properly, not C in C++. – Some programmer dude Dec 09 '22 at 09:28
  • I am required to use the realloc as a specification for this project. I agree that using vector would be much easier as well as using new. – ibucksss Dec 09 '22 at 09:35
  • Is this the *complete* code??? There seems to be a free-standing `if` and your `array_creation` function isn't called anywhere. As is, the code won't compile at all. Please create a [mre]! – Aconcagua Dec 09 '22 at 09:45
  • About the `std::vector` – what exactly are your requirements? Note that you might at least use a `std::vector` to store the manually allocated pointer arrays – and the `int**` pointer (if you e.g. need to pass it to some C API) you can get from via `myVector.data()` (or `&myVector[0]` for pre-C++11 code)! Only exception to: If the `int**` pointer is passed to some data sink that is going to clean it up by use of `free`, of course. – Aconcagua Dec 09 '22 at 10:08

2 Answers2

0

Look at your malloc

int ** arr = (int **) malloc(buffer_size * sizeof(int *));

Now look at your realloc

arr = (int **) realloc(arr, buffer_size * sizeof(int));

See the difference? It should be sizeof(int *) for both cases.

john
  • 85,011
  • 4
  • 57
  • 81
  • 1
    This is still not correct usage of `realloc`! – Aconcagua Dec 09 '22 at 09:25
  • @Aconcagua How so? – john Dec 09 '22 at 09:26
  • 1
    I would guess their point is that it relies on the realloc succeeding. The proper way is to store the result in another variable and null-check it before overwriting your original. – paddy Dec 09 '22 at 09:32
  • 2
    OK noted, although I think it's also perfectly reasonable to ignore the possibility of memory exhaustion. – john Dec 09 '22 at 09:34
  • @john This argumentation leads to undefined behaviour on dereferencing the nullpointer. On modern OS this would most likely lead to crash (segmentation fault), on MCU this can actually be a valid address, then usually the start address of your programme. You'd render your MCU unstartable if writing a bad value to... – Aconcagua Dec 09 '22 at 09:41
  • @john -- but the original code doesn't ignore the possibility of memory exhaustion; it checks the value returned by `malloc` consistently. – Pete Becker Dec 09 '22 at 14:34
  • @PeteBecker The original code attempts to print an error message when `malloc` returns NULL but then goes ahead and uses the pointer anyway, thus invoking undefined behaviour and invalidating the entire program. I don't think you can read too much into the OPs code, since they are clearly a beginner, – john Dec 09 '22 at 14:54
  • @john -- nevertheless, the code does not "ignore the possibility of memory exhaustion", even though it doesn't handle it sensibly. – Pete Becker Dec 09 '22 at 17:18
0

Correct usage of realloc (in general) looks as follows; note that you need a temporary variable to hold the result of the reallocation because you'd lose the old pointer on failure otherwise:

auto tmp = realloc(thePointer, newSize);
if(tmp)
{
    // cast here or earlier, doesn't matter
    thePointer = static_cast<decltype(thePointer)>(tmp);
}
else
{
    // appropriate error handling; note how `thePointer` remainded
    // unchanged and you still have access to the previous array,
    // so you might e.g. free it to avoid memory leaks
}

In your specific case the pointer type is int**, the new size is buffer_size * sizeof(int*) (note that you missed an asterisk there in your code!).

However usage of malloc/realloc is discouraged in C++. You avoid all this memory management hassle if you use std::vector instead of raw arrays.

Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • Thank you for the help. When I tried to update that line to arr = (int **) realloc(arr, buffer_size * sizeof(int *)); it still is giving me the Segmentation Fault. Thanks again for the help, like I said I am required to use the realloc on it and I know it is not best. – ibucksss Dec 09 '22 at 09:38
  • @ibucksss You haven't provided a [mre] – in these little fragments you provided there's too much lacking. A complete working example might look like [this](https://godbolt.org/z/58xae9d8b) – note, though, that, instead of allocating all sub-arrays immediately, it would be better to initialise all positions in `arr` to `nullptr` instead and only `malloc` such a sub-array on increasing `buffer_count`. – Aconcagua Dec 09 '22 at 10:04