String duplicates when printed to text file

Question

I'm trying to write a sentence string to a text file, but it duplicates or something when I do so. Why is this, and how do I get it to only print once?

Program excerpt:

FILE *memberAdd;

typedef struct {
char id[5], name[100];
} Member;
Member reg;

strcpy(reg.id, "M0001");
printf("Name: ");
rewind(stdin);
scanf("%[^\n]", reg.name);

memberAdd = fopen("member.txt", "a");
fprintf(memberAdd, "%s %s\n", reg.id, reg.name);
fclose(memberAdd);

Output in text file when I run the above (reg.name input is Test Name):

M0001Test Name Test Name

Printing `reg.id` outputs `M0001Test Name` because of buffer overflow. — huysentruitw, Dec 21 '17 at 10:40
'char id[5]' WHY? Please stop this pointless, bug-attracting, unjustifiable bean-counting unless you have a RAM-restricted embedded environment. Put in [128] and be done with. — Martin James, Dec 21 '17 at 11:49
Valgrid might be able to show you why `strcpy(reg.id, "M0001");` doesn't play nicely with `char id[5]`; in any case, you're missing some important library headers and a `main()` function - please add them to make a [mcve]. — Toby Speight, Dec 21 '17 at 14:17

score 2 · Answer 1 · answered Dec 21 '17 at 10:39

This:

char id[5];

strcpy(reg.id, "M0001");

is a buffer overflow, strpcy() will write the 0-termination character at the end, overflowing the ìd array. You're getting undefined behavior. To fix it make the ID string shorter, or increase the size of the array of course.

Sourav Ghosh · Accepted Answer · 2017-12-21T10:52:12.903

In your code

  strcpy(reg.id, "M0001");

is off-by-one. you cannot store a string like "M0001" (5 chars and a null-terminator) in an array of 5 chars using strcpy(). You're accessing beyond the allocated memory, this causes undefined behavior.

Quoting C11, chapter §7.24.2.3

The strcpy function copies the string pointed to by s2 (including the terminating null character) into the array pointed to by s1. [...]

So, it's implied, that the destination must be able to hold the source string, including the null terminator. You're running short of memory here, it's proved. :)

You need to have the memory to store the null terminator also, if you plan to use a char array as a string. In this case, you can increase the member id dimension to hold the terminating null, like

 char id[6];

score 1 · Answer 3 · answered Dec 21 '17 at 10:40

Every C string needs to be terminated by some NUL byte (i.e. (char)0).

So "M0001" takes six (not 5!) bytes. And your ~~char id[5]~~ field is not wide enough. You get a buffer overflow, an instance of undefined behavior. You should have declared

 char id[6];

at least as a field in Member. I actually recommend making it bigger, perhaps char id[8];

String duplicates when printed to text file

3 Answers3