Swap pointers instead of memcpy

EDIT: I'm sorry for my mistakes in my code snippets, now I see both outputs were same. Below is an edited version.

Let's say I have a structure:

typedef struct
{
    char m[5];
    char f[6];
} COUPLE;

And a file containing just phrase RomeoJuliet that I read into an array:

char *data = malloc(11);
FILE *f = fopen("myfile", "rb");
fread(data, 1, 11, f);
fclose(f);

I always use this code when I need to fill my structure from a byte array:

COUPLE titanic;
memcpy(&titanic, data, sizeof(data));
printf("%s and %s", titanic.m, titanic.f);

This works fine, but really my byte array can be very big, so below is my attempt to optimize my code.

COUPLE *titanic = (COUPLE *)data;
printf("%s and %s", titanic->m, titanic->f);

So, my questions are:

1. (obsolete) Why do I get different outputs?
2. (obsolete) How can I fill a structure just by casting from an array?
3. Should I avoid this kind of optimization?
4. Are there possible pit开发者_开发问答falls in it?

---

What your are saying is not true. You method if filling a struct from a string my using memcpy will not and does not work. Firstly, when you do

memcpy(&titanic, data, sizeof(data));

you are copying 12 bytes from the char array. Meanwhile, the struct is not guaranteed to be 12 bytes large (the total size is only 11), so it will cause memory overrun in general case.

Secondly, the char array in titanic.m will not be zero terminated. The same is true for titanic.f, which is why your printf has absolutely no change of outputing "Romeo and Juliet", as you falsely claim.

You claim that your first example "works fine" (why?), while in reality the first example does not work at all, and even when it "works" it suffers from the very similar problems to your second example (although in general case the exact problems will not be predictable).

The approach you are trying to use is not viable. You can't produce a correctly formed struct with two strings in it from one original string. One obvious reason why it can't be done is that for two strings you need two zero-terminators, while your original string has only one.

---

When made my comment to your question, I didn't have the time to elaborate, so here's an attempt to answer. The code has changed since, and I'm not sure I would add that comment to the code as it is now.

Nevertheless, the underlying reason that made me add that comment still stands: Unless you measured and found that the code in question does indeed have a significant negative impact on performance, don't attempt to optimize. Instead, strive to make your code as readable as possible.

I seriously doubt that copying the data in memory will have a significant performance impact after copying them from the disk into memory. However, since your code as provided makes assumptions about the struct's layout in memory anyway, directly reading into the struct wouldn't really make the code less readable (or less vulnerable to changes to that layout):

COUPLE titanic;
FILE *f = fopen("myfile", "rb");
fread(&titanic, sizeof(titanic), 1, f);

Or, if you indeed have an array, read into the array directly:

COUPLE titanic[SIZE];
FILE *f = fopen("myfile", "rb");
fread(&titanic, sizeof(titanic), SIZE, f);

Depending on SIZE, the latter could indeed make a potentially huge difference in performance. Accessing the disk for bigger chunks is, in general, faster that doing it for smaller chunks. (Although disk caching might alleviate that.)

---

1. Because it's the members start at a different position. Printf() print a null (\0) terminated sentence.
2. You can do it like that. But you need to pay attention to the types and how to use them.
3. It can be good if you know what you're doing.
4. Lot's of pitfalls if you don't know what you're doing... There's no type safety.

---

I think that the only solution that doesn't involves memcpy would be solution that directly read from file to structure. This could be something like this

typedef struct
{
    char m[6];
    char f[7];
} COUPLE;

COUPLE titanic;

FILE *f = fopen("myfile", "rb");
fread(titanic.m, 1, 5, f);
titanic.m[5] = '\0';
fread(titanic.f, 1, 6, f);
titanic.f[6] = '\0';
fclose(f);

printf("%s and %s", titanic.m, titanic.f);

Of course this example probably is not optimal, because I think that one call of fread and few memcpy would be faster than 2 freads. That way or another both solutions are very unrealistic, because the problem is unrealistic - who needs structure that stores 5 and 6 character length name.