CODEWITHSUNDEEP
cparsinginterpreter
theanine
theanine

Reputation: 1008

Better practice to strcpy() or point to another data structure?

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?

Upvotes: 4

Views: 583

Answers (7)

salchams
salchams

Reputation: 39

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.

Upvotes: -1

Jens Gustedt
Jens Gustedt

Reputation: 78993

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

thing.label = strdup(pair.elementName);

Upvotes: 0

Michael Burr
Michael Burr

Reputation: 340506

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.

Upvotes: 1

rayners
rayners

Reputation: 539

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).

Upvotes: 0

zvrba
zvrba

Reputation: 24584

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.

Upvotes: 3

Michael Mrozek
Michael Mrozek

Reputation: 175735

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)

Upvotes: 1

Mark Wilkins
Mark Wilkins

Reputation: 41262

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.

Upvotes: 0

Related Questions