0

This is a snippet of code that I took from a bigger one. I need to figure out whether or not what I'm printing is garbage and how to change it for it to print the value I need. I need it to print the value of int id instead of whatever it's printing. The output was 10813776 in this run and it, of course, changes whenever I change some code or relaunch DevC++.

The code is:

#include <iostream>
#include <memory> //Memory allocation (malloc)

using namespace std;

int main(){
    int id = 0;
    int nMemory = 0;
    int * pMemory;
    pMemory = (int *) malloc(20 * sizeof(int));
    
    while(id < 20){
        if (pMemory != NULL){
            cout << "Numbers in memory: " << endl;
            pMemory = new int[id];
            nMemory = *pMemory;
            cout << nMemory << endl;
        }
        id++;
    }
    delete pMemory;
    
    return 0;
}
Daniel Pantigoso
  • 35
  • 7
  • Don't use `malloc()` in C++. Use operator `new` – Jorengarenar Jul 14 '20 at 03:31
  • I'm fairly new to C++. I know that I'll only need 20 integers, so that's why I used it. Any advantage to using ```new```? How would I implement it? – Daniel Pantigoso Jul 14 '20 at 03:34
  • `new int[id]` is semi-"equivalent" to `malloc(sizeof(int) * id)` – Jorengarenar Jul 14 '20 at 03:35
  • 1
    *I'm fairly new to C++* -- If this is the case, where did you get the idea to use `malloc` in a C++ program? No C++ book I know of shows this. It is undefined behavior to allocate memory using `malloc`, and deallocate it using `delete`. – PaulMcKenzie Jul 14 '20 at 03:36
  • @PaulMcKenzie I read 'A Tour of C++' second edition by Bjarne Stroustrup, which was recommended by Derek Banas, whose [tutorial](https://youtu.be/6y0bp-mnYU0?t=6150) I'm following. He used malloc. – Daniel Pantigoso Jul 14 '20 at 03:41
  • @RemyLebeau I just need a simple code that allocates 20 blocks of memory (```int``` sized), writes a value to it, and then prints the values stored in those blocks. I am aware it is overall messy, although unaware of its bad behaviors and why. I'd rewrite it completely, were I knowledgeable enough to do it. I understood what he was doing, but I need something that goes a step further. Clearly, I don't know how to do it. – Daniel Pantigoso Jul 14 '20 at 03:52
  • 1
    Tour's good, but it's best used as a refresher. The preference order when allocating storage should go Automatic, [library container](https://en.cppreference.com/w/cpp/container), [smart pointer](https://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one), `new`, `malloc`. About the only time you should pull out `malloc` is really low-level scaffolding. – user4581301 Jul 14 '20 at 03:53
  • @DanielPantigoso It is not at all clear what this code is trying to accomplish. This code is just a jumble of messiness and bad behaviors. It needs to be completely rewritten. Whatever you took away from that tutorial is not what it was trying to teach you. – Remy Lebeau Jul 14 '20 at 03:54
  • 1
    @DanielPantigoso -- If memory is allocated using `malloc` it is deallocated using `free`. If memory is allocated with `new`, it is deallocated with `delete`. If memory is allocated with `new[]`, it is deallocated with `delete[]`. If you mix these pairs up, you are invoking undefined behavior. – PaulMcKenzie Jul 14 '20 at 03:57
  • 1
    Think of `malloc` and `new` in a single C++ source as peanut-butter and barbecue sauce. Don't mix them -- they don't go together. – David C. Rankin Jul 14 '20 at 04:03

2 Answers2

2
pMemory = new int[id];
nMemory = *pMemory;

The first line replaces the array you've malloc-ed with a new uninitialized one, and then tries to read from the first slot of that new array. You shouldn't be assigning directly to pMemory; maybe to pMemory[someIndex], but not to pMemory itself.

Are you trying to read from the pMemory array and assign it to nMemory? If so, change the lines above to this:

nMemory = pMemory[id];

Your entire loop should look more like this:

if (pMemory != NULL) {
    cout << "Numbers in memory: " << endl;
    while(id < 20) {
        nMemory = pMemory[id];
        cout << nMemory << endl;
        id++;
    }
}

Or, with a more idiomatic for loop:

if (pMemory != NULL) {
    cout << "Numbers in memory: " << endl;
    for (int i = 0; i < 20; i++) {
        cout << pMemory[i] << endl;
    }
}

(You're also going to have to initialize the array somewhere above this loop. I presume you do that in your real code, but if not: the code you posted allocates an array with malloc() but does not set the items to useful values. Make sure to set them to something meaningful before you then try to read and print them.)

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
  • My code used ints from a text file and wrote them to the memory, but it still prints out garbage. In all honesty, I don't know where exactly it's going wrong. – Daniel Pantigoso Jul 14 '20 at 03:46
  • @DanielPantigoso nothing in the code you showed is filling the allocated memory with any data at all, let alone from a file. Please update your question to show a [mcve] demonstrating your real code. – Remy Lebeau Jul 14 '20 at 03:56
2

This code is leaking the memory blocks you allocate with malloc() and new[].

You malloc() a block of memory and assign its address to pMemory, then you change pMemory to point at different memory addresses that are allocated with new[]. So you lose the ability to free() the malloc()'ed memory (you are not even trying to call free()).

And, this code is not freeing the memory allocated with new[] correctly. Memory allocated with new[] must be freed with delete[], not with delete. Worse, you are calling new[] 20 times in a loop, but calling delete only once after the loop. So, you are leaking 19 blocks of new[]ed memory, and have undefined behavior freeing 1 block.

Now, to answer your actual question, the code is printing out garbage because the memory you are allocating with new[] is uninitialized, so the data you are trying to print from that memory contains indeterminate values.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770