How do I appropriately set the pointer to a new structure?

Question

I have a question regarding initialization of a new structure in c++. I am new to c++.

typedef struct 
{
   int n;
   char anArray*;
}  myStruct;

void newStruct ( myStruct **ms, int x)
{
   myStruct* local_ms = new myStruct;
   local_ms->n = x;
   local_ms->anArray= new char[sizeof(char)*n];
   ms = &local_ms;
}

When I call newStruct with an void pointer, my intention is for it to allocate the memory in myStruct and then store the pointer to the new structure in ms, for my use later. Unfortunately, I think local_ms is only local in scope and is thus lost upon return from newStruct.

That being said, I'm not sure how to appropriately initialize myStruct! Thoughts?

If you're new to C++, now would be a great time to [read a book](http://stackoverflow.com/questions/388242) and stop writing as if it were C. — Mike Seymour, Sep 10 '13 at 10:52
Please avoid the c-style of passing things in pointers (and avoid pointers in general) — , Sep 10 '13 at 10:56
The only thing I'm seeing here about C++ language is `new` instead of `malloc`. Its sadly that I cannot modify the tags of the question to `C` because this implies I have to modify the code of the question. — Manu343726, Sep 10 '13 at 12:19

score 1 · Answer 1 · answered Sep 10 '13 at 10:59

1

"I think local_ms is only local in scope and is thus lost upon return from newStruct."

Change:

ms = &local_ms;

to:

*ms = local_ms;

would help avoid the problem, which assigns the pointer of the newStruct object to the *ms.

answered Sep 10 '13 at 10:59

lulyon

6,707
7
32
49

Mike Seymour · Accepted Answer · 2013-09-10T13:14:21.813

ms = &local_ms;

This modifies the local pointer ms to point to the other local pointer to the allocated structure. However, what you want is to modify the caller's pointer. ms is a pointer to that, so you want to modify the thing that ms points to:

*ms = local_ms;

But this isn't C, so you could use simpler reference semantics:

void newStruct ( myStruct *& ms, int x)
//                        ^^ reference to pointer
{
    // ...
    ms = local_ms;
}

// usage
myStruct * ms;
newStruct(ms, 42);

But the language (C or C++) gives a cleaner way to return a value from a function: you can return a value from the function.

myStruct * newStruct(int x)
{
    // ...
    return local_ms;
}

// usage
myStruct * ms = newStruct(42);

But in C++, we can use constructors, rather than arbitrary functions, to initialise new objects:

struct myStruct {              // no need for that typedef nonsense
    explicit myStruct(int n) : 
        n(n), 
        anArray(new char[n])   // sizeof(char) is 1 by definition
    {}

    int n;
    char *anArray;             // * goes before the variable name
};

// usage
myStruct ms(42);               // don't use `new` unless you really need it

Now just one thing is missing: anArray is never deleted, giving a memory leak. The simplest fix is to use a dynamic array type from the standard library: either string or vector.

struct myStruct {
    explicit myStruct(int n) : n(n), anArray(n) {}

    int n;
    std::string anArray;
};

But of course, n is now redundant; you should get rid of it and use anArray.size() instead. This means that the structure itself is pretty pointless; you just need

std::string ms(42);

Thank you. That seemed to work. What if anArray was more complicated than a simple character array (string). Say, a pointer to another object? How would you correct the memory leak then? — user2764533, Sep 10 '13 at 22:23

score 0 · Answer 3 · answered Sep 10 '13 at 12:56

#include <memory>
#include <iostream>

// [A] Starting with:

typedef struct
{
   int n;
   // Not char anArray*;
   char* anArray;
}  myStruct;

void newStruct ( myStruct **ms, int x)
{
   myStruct* local_ms = new myStruct;
   local_ms->n = x;
   // Fix: use x
   local_ms->anArray = new char[sizeof(char)*x];
   ms = &local_ms;
}

//  [B] Avoid myStruct **ms, use std::size_t
//      and get rid of sizeof(char) (which is one, always)

myStruct* newStruct (std::size_t x)
{
   myStruct* ms = new myStruct;
   ms->n = x;
   ms->anArray= new char[x];
   return ms;
}

// [C] Manage memory in a class (pair new/delete).
//     Btw. typedef struct is C (not C++)

class myStruct2
{
    public:
    myStruct2(std::size_t n)
    :   n(n), anArray(new char[n])
    {}

    ~myStruct2() {
        delete [] anArray;
    }

    std::size_t size() const { return n; }
    const char* array() const { return anArray; }
    char* array() { return anArray; }


    private:
    // If you do not define these, avoid copies (C++11 has '= delete'):
    myStruct2(const myStruct2&);
    myStruct2& operator = (const myStruct2&);

    std::size_t n;
    char* anArray;
};

// Still having a new without delete in the same (logically) scope - which is bad:
myStruct2* newStruct2 (std::size_t n)
{
   return new myStruct2(n);
}

// [D] Manage memory with a shared pointer.

// Still having an new without a delete in the same (logically) scope,
// but now the memory is managed by the shared pointer - that is good!
// (If there is no std::shared_ptr: boost::shared_ptr)
std::shared_ptr<myStruct2> make_shared_struct2(std::size_t n)
{
   return std::shared_ptr<myStruct2>(new myStruct2(n));
}

// [E] Avoid the pointer to myStruct2
// If you have defined the copy constructor and assignment operator:
// myStruct2 make_struct2(std::size_t n)
// {
//    return myStruct2(n);
// }

// [F] But actually it is trivial
typedef std::vector<char> myStruct3;

myStruct3 make_struct3(std::size_t n)
{
    return myStruct3(n);
}

How do I appropriately set the pointer to a new structure?

3 Answers3