C++ Set Array In Pointer to Struct

Question

I have a struct in C++ as follows:

typedef struct DIFDict
{
    std::string name;
    std::string defs[];
    struct DIFColor *colors[];
    uint8_t defsize; 
} DIFDict;

The trouble is trying to initialize it. I have a function which returns a pointer to a struct with all the values initialized:

struct DIFDict *CreateDIFDict(std::string n, std::string *d, struct DIFColor **c, uint8_t de)
{
    int memsize = sizeof(n) + sizeof(d) + sizeof(c) + sizeof(de);
    struct DIFDict *i = static_cast<struct DIFDict*>(malloc(memsize));

    if(i != NULL)
    {
        i->name = n;
        i->defs = d;
        i->colors = c;
        i->defsize = de;
    }

    return i;

}

However, the compiler complains of type mismatches:

error: incompatible types in assignment of 'std::string {aka std::basic_string<char>}' to 'std::string [0] {aka std::basic_string<char> [0]}'
error: incompatible types in assignment of 'DIFColor**' to 'DIFColor* [0]'

Am I misunderstanding the relationship between pointers and arrays?

Don't mix C with C++. call malloc for string will not construct the string object. — billz
– billz, Commented Jul 28, 2014 at 23:31
malloc() doesn't call default constructors. Use new instead. — David G
– David G, Commented Jul 28, 2014 at 23:42
This struct definition is illegal in both C and C++. C++ does not permit [] anywhere in the struct definition; and C only permits it as the final member of a struct. (notwithstanding the fact that std::string does not exist in C) — M.M
– M.M, Commented Jul 28, 2014 at 23:50

PaulMcKenzie · Accepted Answer · 2014-07-29 00:51:18Z

2

typedef struct DIFDict
{
    std::string name;
    std::string *defs;
    struct DIFColor **colors;
    uint8_t defsize; 
} DIFDict;

struct DIFDict *CreateDIFDict(std::string n, std::string *d, struct DIFColor *c, uint8_t de)
{   
    DIFDict *i = 0;
    i = new DIFDict;

    if(i != 0)
    {
        i->name = n;
        i->defs = d;
        i->colors = &c;
        i->defsize = de;

        return i;
    }

    return 0;
}

Edit: The issue with the answer above is that the DIFColor parameter is a temporary, and pointer to the temporary will be invalid when the function exits. A more viable (not the best, but at least viable) solution that closely mimics the above would be as follows:

    struct DIFDict
    {
        std::string name;
        std::string *defs;
        DIFColor **colors;
        uint8_t defsize; 
    };

    DIFDict *CreateDIFDict(const std::string& n, std::string *d, 
                           DIFColor **c, uint8_t de) 
    {   
        DIFDict *i = new DIFDict;
        i->name = n;
        i->defs = d;
        i->colors = c;
        i->defsize = de;
        return i;
    }

Note that we do not now take the address of the temporary -- all we're passing the pointer value directly and having it assigned. Also, note that new does not return 0 on failure (unless the nothrow option is used), so no check is needed for new being 0.

However, this solution is still C-like and is error-prone, and there are much better methods, as outlined by the other answers given.

edited Jul 29, 2014 at 0:51

PaulMcKenzie

35.6k4 gold badges26 silver badges49 bronze badges

answered Jul 28, 2014 at 23:44

silent

2,92411 gold badges49 silver badges73 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

M.M Over a year ago

c is an automatic variable, so i->colors is a wild pointer as soon as this function returns

M.M · Accepted Answer · 2014-07-29 00:10:19Z

You seem to have some basic misunderstanding of arrays in C++. They have have a fixed size, known at compile-time. Zero-sized arrays are illegal. (Some compilers allow a zero-sized array when not invoked in standard mode, however they have unclear semantics, and certainly can't have any value stored in them).

Also you use sizeof wrongly. sizeof x tells you how many bytes are required to store x. (NOT anything that x may be pointing to, if x is a pointer, and not anything that x is a handle for, or a container class of). For example std::string is a container which has a pointer pointing to dynamically-allocated string contents. sizeof name only gets you the container size, it does not get you how much space has been dynamically allocated.

In your code, memsize can be determined at compile-time; it would never be different on different calls to the function.

Further, using a function CreateDIFDict is poor style as it forces dynamic allocation. Instead, provide a constructor. Then the user can choose whether they want dynamic allocation, and they can take advantage of move and copy semantics.

It's not clear whether or not you want DIFDict to take a copy of the strings and DIFColors that you are initializing it with. If you do not want a copy:

struct DIFDict
{
    std::string name;
    std::string *defs;
    struct DIFColor **colors;
    uint8_t defsize; 

    DIFDict(std::string name, std::string *defs, struct DIFColor **colors, uint8_t defsize) :
         name(name), defs(defs), colors(colors), defsize(defsize) {}
};

Make very sure that you destroy all DIFDicts before you destroy the things that the pointers are pointing to. (Raw pointers do not have an ownership semantic in C++).

If you do want a copy:

struct DIFDict
{ 
    std::string name;
    std::vector<std::string> defs;    // or some other container
    // ??? colors

    DIFDict(std::string name, std::string const *p_defs, uint8_t defsize)
         : name(name), defs(p_defs, p_defs + defsize) {}
};

I'm not sure what colors is supposed to point to, if you give more information about the expected behaviour of colors I'll update this example.

jww · Accepted Answer · 2014-07-29 00:01:05Z

1

This is C++, not C. Drop the typedef and initialize it as you would a class. The only difference between a struct and a class is default visibility of the members. Something like:

struct DIFDict
{
    string name;
    vector<string> defs;
    DIFColor* colors;
    uint8_t defsize; 

    DIFDict();
    ~DIFDict();    
};

DIFDict::DIFDict : name(""), defs(...), colors(...), defsize(0) {}
DIFDict::~DIFDict() {...}

And as 0x499602D2 points out below, you should probably use a vector for your arrays.

The ellipses mean to create a helper to initialize the members. The helper should ensure the constructor behaves as expected during an exception.

edited Jul 29, 2014 at 0:01

answered Jul 28, 2014 at 23:37

jww

104k107 gold badges453 silver badges975 bronze badges

7 Comments

Blindy Over a year ago

Completely irrelevant.

David G Over a year ago

Don't even need the second struct and that array of pointers can be changed into a std::vector or std::array of std::unique_ptr.

jww Over a year ago

Blindy - forgive my ignorance, but how is it irrelevant? The question is labeled C++, and he's trying to initialize his struct: "... the trouble is trying to initialize it". What would you use?

user657267 Over a year ago

This is C++ Zero-size arrays are non-standard.

Igor Over a year ago

I'm actually thinking about using a map now, since each def would match a color

|

Collectives™ on Stack Overflow

C++ Set Array In Pointer to Struct

3 Answers 3

1 Comment

Comments

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related