views:

94

answers:

7

Because it's always easier to see code...

My parser fills this object:

typedef struct pair {
 char* elementName;
 char* elementValue;
} pair;

My interpreter wants to read that object and fill this one:

typedef struct thing {
 char* label;
} thing;

Should I do this:

thing.label = pair.elementName;

or this:

thing.label = (char*)malloc(strlen(pair.elementName)+1);
strcpy(thing.label, pair.elementName);

EDIT: Yes, I guess I should have specified what the rest of the program will do with the objects. I will eventually need to save "pair" to a file. So when thing.label is modified, then (at some point) pair.elementName needs to be modified to match. So I guess the former is the best way to do it?

A: 

From an "object" independence standpoint, it is probably better to make a copy of the data to avoid problems with dangling pointers.

It would be more efficient and faster to just assign the pointer, but unless that extra performance is highly critical, you will probably be better off (from a debugging standpoint) by making the copy.

Mark Wilkins
+1  A: 

I would personally do the former, but it's a tradeoff. The former avoids the need to allocate new memory and copy data to it, but the latter avoids the confusion of aliasing by keeping thing.label and pair.elementName pointing to separate memory addresses, which means you need to free both of them (with the former you need to be sure to free exactly one, to avoid either a memory leak or a double free)

Michael Mrozek
You convinced me to copy the memory. Thanks.
ComtriS
+3  A: 

No good answer to that question as there is too little context. It all depends on how the rest of the program manages the lifetimes of the objects it creates.

zvrba
The objects live throughout the lifetime of the program.
ComtriS
A: 

The answer is, as always, "It Depends." If all you are doing with the "copied" value is reading it, it is probably okay to just copy the pointer address (i.e., the former), as long as you cleanup properly. If the "copied" value is going to be modified in any way, you are going to want to create a new string entirely (i.e., the latter) to avoid any unintended side effects caused by the "original" value changing (unless, of course, that is exactly the desired effect).

rayners
+1  A: 

Here are some of the things that need to be known to answer the question:

  • Which object will 'own' the string? Or will both own their string (in which case a 'deep' copy is necessary)?
  • are the lifetimes of the pair and thing objects related in any way - will one object always 'outlive' the other? Does one of these objects own the other one?

If the pair and thing objects are independent, then copying the string data is probably the correct thing to do. If one is owned by the other, then that might indicate that a simple sharing of the pointer is appropriate.

Not that these are the only possible answers - just a couple of the easier ones.

Michael Burr
A: 

If you want to do a copy, and do all the cleanup afterwards... in C you should do this:

thing.label = strdup(pair.elementName);
Jens Gustedt
`strdup` is nonstandard, though easy to write. A bigger potential problem is repeatedly calculating `strlen` which should be eliminated if possible.
Chris Lutz
You mean not standardized in C? Well it is *only* POSIX, yes.
Jens Gustedt
A: 

I don't want to be a c-police, but please use safer strncpy() instead of strcpy().

char* strncpy(char *s1, const char *s2, size_t n); 

strncpy function copies at most n characters from s2 into s1.

salchams