Copying a struct into an array in C++

Question

I've a struct of the following type

typedef struct Edge
{
    int first;
    int second;

}Edge;

Which I'm instantiating in my main function and copying into an array

Edge h_edges[NUM_EDGES];
for (int i = 0; i < NUM_VERTICES; ++i)
    {
        Edge* e = (Edge*)malloc(sizeof(Edge));
        e->first = (rand() % (NUM_VERTICES+1));
        e->second = (rand() % (NUM_VERTICES+1));
        memcpy(h_edges[i], e, sizeof(e));
    }

I keep running into the following error.

src/main.cu(28): error: no suitable conversion function from "Edge" to "void *" exists

Line 28 is the line where the memcpy happens. Any help appreciated.

The error is because you have an item h_edges[i] of type Edge in memcpy which is expecting a void *. You can't use a struct type in place of a pointer type. You need memcpy(&h_edges[i], e, sizeof(*e)). In other words, the address of h_edges[i] is what you're copying to, and you want the size of what e points to not the size of e itself, which is a pointer (you'd get the size of a pointer). You could also use sizeof(Edge) there. — lurker
– lurker, Commented Dec 4, 2015 at 16:47
And... do not cast the return of malloc. Just use, Edge* e = malloc(sizeof(Edge));. — lurker
– lurker, Commented Dec 4, 2015 at 16:49
If I dont cast the value, this is what I get src/main.cu(25): error: a value of type "void *" cannot be used to initialize an entity of type "Edge *" — Antithesis
– Antithesis, Commented Dec 4, 2015 at 16:51
Not casting the malloc should not generate an error. If it does, there's something else wrong. Edge *e = malloc(sizeof(Edge)); should work just fine. See this post for details on why you shouldn't cast it. — lurker
– lurker, Commented Dec 4, 2015 at 16:53
Note that because you are using the CUDA compiler driver to compile this code in a .cu file, your code is, in fact, being compiled by a C++ compiler, and C++ rather than C semantics apply to the use of malloc. Of course it isn't at all obvious why you are using mallocin that code snippet at all. — talonmies
– talonmies, Commented Dec 5, 2015 at 15:10

Bo Persson · Accepted Answer · 2015-12-04 16:52:13Z

6

You don't need to use malloc or memcpy at all. You can just:

Edge h_edges[NUM_EDGES];
for (int i = 0; i < NUM_VERTICES; ++i)   // or should this be NUM_EDGES??
{
    h_edges[i].first = (rand() % (NUM_VERTICES+1));
    h_edges[i].second = (rand() % (NUM_VERTICES+1));
}

answered Dec 4, 2015 at 16:52

Bo Persson

92.9k31 gold badges153 silver badges211 bronze badges

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

2 Comments

Bathsheba Over a year ago

Yes, this is indeed much better. But do fix the random number selection if you need good uniformity.

lurker Over a year ago

Yes, this is the better approach.

Bathsheba · Accepted Answer · 2015-12-04 16:55:08Z

2

The first parameter of memcpy takes a pointer and the third argument needs to the size of the struct, not the pointer.

memcpy(&h_edges[i], e, sizeof(*e));

is the fix.

But this is ill-advised. Strictly the behaviour of your program is undefined. You can't copy a structure using memcpy in a defined way. Essentially the reason for this lies in structure packing.

Your best bet is to copy the structure members explicitly. You could build a function to do that.

An other point is that using % will introduce statistical bias in your random numbers, unless the modulus is a multiple of the generator's periodicity; which is unlikely.

(A small point, write Edge* e = malloc(sizeof(Edge)); instead. i.e. drop the cast on the right hand side. It's unnecessary in C.)

edited Dec 4, 2015 at 16:55

answered Dec 4, 2015 at 16:50

Bathsheba

235k35 gold badges376 silver badges504 bronze badges

4 Comments

lurker Over a year ago

Needs to be sizeof(*e) since e is a pointer to what's being copied (an Edge structure). Or, sizeof(Edge).

lurker Over a year ago

I don't think the memcpy of the full structure is undefined behavior. C should consistently define the internal structure of the struct just fine. And the type cast on malloc is not only unnecessary, it can cause issues.

Bathsheba Over a year ago

I'm not sure you're correct on that point, I'll check though.

lurker Over a year ago

If you write a function in C that returns a struct, the gcc compiler will internally generate a call to memcpy to copy the struct for the return value. So I think it's good. :)

Collectives™ on Stack Overflow

Copying a struct into an array in C++

2 Answers 2

2 Comments

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related