0

Some help here, trying to double the array in cpp

on the second resize I got this (messy output)

% ./sample
[5] 1 2 3 4 5 
[20] 1 2 3 4 0 6 7 -1610612736 1 2 3 4 14 0 0 0 0 -1610612736 0 0 

the same removing the memset (to zero's the remaining positions)

#include <iostream>
#include <cstdlib>

using namespace std;

void resize(int* v, int* size){ // double the array size
    int newSize = (*size) * 2;
    v = (int *) realloc(v, (size_t) newSize);
    memset(v+(*size), 0, sizeof(int) * (newSize - *size) );
    *size = newSize;
}

int main(){
    int size = 5;
    int* v = new(nothrow) int[size]();

    v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; v[4] = 5;

    cout << "[" << size << "] ";
    for(int i = 0; i < size; i++) cout << v[i] << " "; cout << endl;

    resize(v, &size);
    resize(v, &size);   // <- messy output

    v[5] = 6; v[6] = 7;

    cout << "[" << size << "] ";
    for(int i = 0; i < size; i++) cout << v[i] << " "; cout << endl; 

    return 0;
}

thanks for the help,

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
mvcorrea
  • 101
  • 1
  • 8
  • 1
    Do not tag both C and C++ except when asking about differences or interactions between the two languages. – Eric Postpischil Feb 14 '22 at 02:06
  • Does this answer your question? [Changing address contained by pointer using function](https://stackoverflow.com/questions/13431108/changing-address-contained-by-pointer-using-function) – kaylum Feb 14 '22 at 02:06
  • 1
    `v` is passed by value to the function. So `v` is a local variable in the `resize` function and setting it does not set the caller's variable. See the duplicate post for more details and options for fixing. – kaylum Feb 14 '22 at 02:07
  • 2
    (a) `realloc` and `memset` each take a size in bytes, but you pass `newSize` to one and `sizeof(int) * (newSize - *size)` to the other. `newSize` is already a number of `int`, not a number of bytes, so do not multiple it by `sizeof (int)`. (b) `resize` cannot return a changed value of `v`; its `v` parameter is a local variable initialized from an argument; changing it does not change the argument. (c) I do not think you are supposed to use `realloc` on pointers obtained from `new`, but I do not follow C++ closely. – Eric Postpischil Feb 14 '22 at 02:09
  • If you are coding in C++, you should not try to mix memory allocations with variants of `new` (and released via variants of `delete`) with memory allocated by `malloc()` et al (and released by `free()`). Doing so is, AFAIK, UB — it is certainly not supported in older versions of C++ (and probably still isn't supported in newer versions of C++). In fact, in general, you should not directly manage memory allocation or release in C++. You should use classes that handle it for you safely. – Jonathan Leffler Feb 14 '22 at 02:19
  • 2
    Calling `realloc` on a pointer returned by `new` has undefined behavior. If you really want to do this, use `malloc` or `calloc` to obtain the initial memory, but be aware that this works correctly without further adjustments only for trivial types. Also, both the nothrow version of `new` and `realloc` can return null pointers. You must check for that before proceeding to use the pointers. Why are you using the nothrow variant of `new` anyway? If you don't have a very good reason to do all of this, don't do it in C++. Use `std::vector` instead. – user17732522 Feb 14 '22 at 02:21

2 Answers2

2

Use malloc() with realloc() or new and delete[] - not both

Your immediate problem is you pass a pointer to your function and the function receives a copy -- so any changes made to v in the function are made to a local copy of the pointer (which has its own and very different address than the pointer v in main())

Another problem is mixing new and realloc() which results in undefined behavior. Either use malloc() in main() or manually create and copy in your function using new and call delete[] on the original before assigning the new block.

Since this is C++, you can solve the problem easily by passing a pointer reference to the function (e.g. int*& v) so realloc() operates on the original pointer address instead of a copy.

NOTE: as JL mentioned in the comment, whenever you call realloc() you must assign the result to a temporary pointer which you validate realloc() succeeded before assigning to the original. When (not if) realloc() fails, it will return NULL overwriting your original pointer with NULL leaving that block of memory unable to be freed creating a memory leak.

To use a temporary pointer with realloc(), you would do:

void resize(int*& v, int *size){ // double the array size
    int newSize = (*size) * 2;
    
    /* always realloc with temporary pointer so if realloc fails
     * you don't overwrite your orignal pointer with NULL creating
     * a memory leak.
     */
    int *tmp = (int *)realloc (v, (size_t) newSize * sizeof *v);
    if (!tmp) {   /* validate every allocation */
        perror ("realloc-v");
        return;
    }
    v = tmp;      /* successful realloc(), assign tmp to v */
    
    memset (v+(*size), 0, sizeof(int) * *size );
    *size = newSize;
}

Also, in main() to use realloc() you must originally allocate with malloc() (or calloc(), or realloc(), but not new). Further, guard your loop scopes or if you are not going to guard the scope with {...} then at least properly indent so your intent is clear, e.g.

int main(){
    int size = 5;
    int *v = (int*)malloc(size * sizeof *v);

    v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; v[4] = 5;

    cout << "[" << size << "] ";
    for (int i = 0; i < size; i++)
        cout << v[i] << " ";
    cout << endl;

    resize(v, &size);
    resize(v, &size);

    if (size > 6) {     /* validate size / protect memory bounds */
        v[5] = 6;
        v[6] = 7;
    }
    
    cout << "[" << size << "] ";
    for (int i = 0; i < size; i++)
        cout << v[i] << " ";
    cout << endl; 

    return 0;
}

(you should also add #include <cstring> for memset())

Using new and delete[]

You can leave your allocation in main() as is, if you create a new block in your function with double the size using new, copy values to the new block (and initialize the rest to 0) and call delete[] on the original block before assigning your new block to the original pointer.

In that case you would do something like:

void resize(int*& v, int *size){ // double the array size
    int newSize = (*size) * 2;
    
    /* create a new block of newSize */
    int *tmp = new(nothrow) int[newSize]();
    /* copy/initialize */
    for (int i = 0; i < newSize; i++) {
        if (i < *size)
            tmp[i] = v[i];
        else
            tmp[i] = 0;
    }
    delete[] v;   /* free original block */
    v = tmp;      /* assign tmp to v */
    
    *size = newSize;
}

Example Use/Output

In either case (malloc(), realloc()) or (new, delete[]) the results are the same:

$ ./bin/realloc_arr
[5] 1 2 3 4 5
[20] 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 0 0 0 0 0

In both cases you will want to verify your memory use with valgrind or similar memory use/error check for your OS. You will need to add either free(v) or delete[] v at the end of main() to handle freeing all memory you allocate for the purpose of the memory check (it will be freed after the program exits, but without an explicity free in main() it will still be shown as in use on program exit.

To use valgrind, just run your program through it. After making the changes above, you would have:

$  valgrind ./bin/realloc_arr
==22674== Memcheck, a memory error detector
==22674== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==22674== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==22674== Command: ./bin/realloc_arr
==22674==
[5] 1 2 3 4 5
[20] 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 0 0 0 0 0
==22674==
==22674== HEAP SUMMARY:
==22674==     in use at exit: 0 bytes in 0 blocks
==22674==   total heap usage: 5 allocs, 5 frees, 73,868 bytes allocated
==22674==
==22674== All heap blocks were freed -- no leaks are possible
==22674==
==22674== For counts of detected and suppressed errors, rerun with: -v
==22674== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always ensure all blocks are freed and there are no memory errors.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
0

for reference the working version

#include <iostream>
using namespace std;

void resize(int*& v, int* size){ // double the array size
    int newSize = (*size) * 2;
    int* tmp = new(nothrow) int[newSize]();  // zeroing on the next line is redundant?
    for(int i = 0; i < newSize; i++) tmp[i] = (i < *size) ? v[i] : 0;
    delete[] v;
    v = tmp; 
    *size = newSize;
}

int main(){
    int size = 5;
    int* v = new(nothrow) int[size]();

    v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; v[4] = 5;

    cout << "[" << size << "] ";
    for(int i = 0; i < size; i++) cout << v[i] << " "; cout << endl;

    resize(v, &size);
    resize(v, &size);   // <- NOW, working output
    //resize(v, &size);

    v[5] = 6; v[6] = 7;

    cout << "[" << size << "] ";
    for(int i = 0; i < size; i++) cout << v[i] << " "; cout << endl; 

    return 0;
}
mvcorrea
  • 101
  • 1
  • 8