strcpy is merging numeric format with other chars

Question

I don’t know if I made myself clear in the title, but when using strcpy() to copy a char* to another when I put a format like this "teste" it works normally, but when I put a string 3 letter format (digits in case), for example "2000" it ends up merging this value for the destination with the next value the next time I use strcpy(), follows the code:

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

typedef struct
{
    char nome[80];
    char ano[4];
    char diretor[80];
} Filme;

void analise(Filme *filme, const char *arg1, const char *arg2, const char *arg3)
{
    Filme _filme = {
        .nome    = malloc(2),
        .ano     = malloc(2),
        .diretor = malloc(2)
    };

    strcpy(_filme.nome, arg1);
    strcpy(_filme.ano, arg2);
    strcpy(_filme.diretor, arg3);

    memcpy(filme, &_filme, sizeof _filme);
}


void carregar(Filme filmes[])
{
    analise(&filmes[0], "E o Vento Levou", "1939", "Victor");
    analise(&filmes[1], "teste", "998", "bar");
    analise(&filmes[2], "Os Passaros", "1963", "Alfred Hitchcock");
}

int main()
{
    Filme filmes[1000];

    carregar(filmes);

    printf("\nmain:\n");

    printf("- Nome:    %s\n", filmes[0].nome);
    printf("- Ano:     %s\n", filmes[0].ano);
    printf("- Diretor: %s\n", filmes[0].diretor);

    printf("----------------\n");

    printf("- Nome:    %s\n", filmes[1].nome);
    printf("- Ano:     %s\n", filmes[1].ano);
    printf("- Diretor: %s\n", filmes[1].diretor);

    printf("----------------\n");

    printf("- Nome:    %s\n", filmes[2].nome);
    printf("- Ano:     %s\n", filmes[2].ano);
    printf("- Diretor: %s\n", filmes[2].diretor);

    return 0;
}

Notice I ran this:

analise(&filmes[0], "E o Vento Levou", "1939", "Victor");
analise(&filmes[1], "teste", "998", "bar");
analise(&filmes[2], "Os Passaros", "1963", "Alfred Hitchcock");

When running the problem the output is this:

main:
- Nome:    E o Vento Levou
- Ano:     1939Victor
- Diretor: Victor
----------------
- Nome:    teste
- Ano:     998
- Diretor: bar
----------------
- Nome:    Os Passaros
- Ano:     1963Alfred Hitchcock
- Diretor: Alfred Hitchcock

See that in "Gone with the Wind" and "Gone with the Birds" the years were merged with the director’s name, 1939Victor and 1963Alfred Hitchcock, already in the case of:

analise(&filmes[1], "teste", "998", "bar");

It has the correct output. I understand that I should make the year with int, but I’m learning C and I’d like to better understand this part of the memory, I assume it was some typo of mine.

Answer 1

This code has some problems.

• There’s nothing to wear malloc() if the area that the string should be already reserved within the structure. It could even allocate if desired, and maybe makes sense for the names, but then you need to declare as const char * and not [tamanho]. You have to be much more careful when you do that. What’s even leaking memory. At high volume that would be tragic. And allocating only 2 characters where you need several also does not work very well, is that in this case coincidentally works.
• Not that it’s a problem, but I see no reason to create a local structure, initialize its members and then copy to the array. Write directly to array, without creating anything intermediate, initialize or copy.
• As strings have a terminator, so you need to make room for it. The specific problem you’re encountering is that it has 4 bytes reserved for the year, so the 4 characters of the year are placed there, and a 5th. is placed next. When you name the director, its first character goes on top of the terminator of the string of the year. Then when it will read the year it has no end, only finalize at the end of the name of the director, so it is all together. If it had 5 bytes, the terminator would be preserved and everything would work ok.

That’s how it works:

#include <string.h>
#include <stdio.h>

typedef struct {
    char nome[81];
    char ano[5];
    char diretor[81];
} Filme;

void analise(Filme *filme, const char *arg1, const char *arg2, const char *arg3) {
    strcpy(filme->nome, arg1);
    strcpy(filme->ano, arg2);
    strcpy(filme->diretor, arg3);
}

void carregar(Filme filmes[]) {
    analise(&filmes[0], "E o Vento Levou", "1939", "Victor");
    analise(&filmes[1], "teste", "998", "bar");
    analise(&filmes[2], "Os Passaros", "1963", "Alfred Hitchcock");
}

int main() {
    Filme filmes[1000];
    carregar(filmes);
    printf("\nmain:\n");
    for (int i = 0; i < 3; i++) {
        printf("- Nome:    %s\n", filmes[i].nome);
        printf("- Ano:     %s\n", filmes[i].ano);
        printf("- Diretor: %s\n", filmes[i].diretor);
        printf("----------------\n");
    }
}

Behold working in the ideone. And in the repl it.. Also put on the Github for future reference.

Answer 2

What happens is that your struct occupies 164 bytes in memory:

• 80 bytes for the name, including the null terminator;
• 4 bytes for the year, you are not considering the null terminator here;
• 80 bytes for director, including null terminator.

The compiler will assign the following shifts for each of these fields:

• nome: 0 bytes offset since the beginning of the struct in memory.
• ano: 80 bytes offset from the beginning of the struct in memory.
• diretor: 84 bytes offset since the beginning of the struct in memory.

So, if we ask the first case for example, the layout looks like this (where is the null terminator and is garbage, which can have any value):

E o Vento Levou∅□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□1939Victor∅□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□□

When you do that:

printf("- Ano:     %s\n", filmes[0].ano);

At runtime, the generated code will load the memory address of filmes, add 164 0 to get filmes[0] (164 = sizeof Filme, 0 is the index) and then add 80 to get the position of filmes[0].ano (80 is the position of ano within the struct). From this position it will print a string (due to the %s) starting at that calculated position and ends at the first null terminator found. It turns out that the year does not have a null terminator, and therefore it will end up invading the subsequent memory region of the field diretor.

Besides, that code makes no sense:

Filme _filme = {
    .nome    = malloc(2),
    .ano     = malloc(2),
    .diretor = malloc(2)
};

What you probably wanted was this:

void analise(Filme *filme, const char *arg1, const char *arg2, const char *arg3) {
    strcpy(filme->nome, arg1);
    strcpy(filme->ano, arg2);
    strcpy(filme->diretor, arg3);
}

To solve your problem without using int for the year, one possibility is to change the size of the field ano to 5. If the other fields should have a maximum of 80 characters instead of 79, then change their size to 81 in order to make sure that the null terminator is there.

Another alternative that avoids having to resize the size of the fields is to specify a maximum size for the strings in the printf:

printf("- Nome:    %.80s\n", filmes[0].nome);
printf("- Ano:     %.4s\n", filmes[0].ano);
printf("- Diretor: %.80s\n", filmes[0].diretor);