0

I have a program that reads the content of a file and saves it into buf. After reading the content it is supposed to copy two by two chars to an array. This code works fine if I'm not trying to read from a file but if I try to read it from a file the printf from buffer prints the two chars that I want but adds weird characters. I've confirmed and it's saving correctly into buf, no weird characters there. I can't figure out what's wrong... Here's the code:

char *buffer = (char*)malloc(2*sizeof(char));
char *dst = buffer;
char *src = buf;
char *end = buf + strlen(buf);
char *baby = '\0';
while (src<= end)
{
    strncpy(dst, src, 2);
    src+= 2;
    printf("%s\n", buffer);
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
ninjacow55
  • 27
  • 1
  • 2
  • 10
  • Try allocating `3*sizeof(char)` instead of `2*sizeof(char)` (+1 for the `\0`) and don't forget to assign a `\0` at the end of `buffer` – Spikatrix Nov 24 '14 at 10:42

1 Answers1

1

  1. (char*)malloc(2*sizeof(char)); change to malloc(3*sizeof*buffer); You need an additional byte to store the terminating null character which is used to indicate the end-of-string. Aslo, do not cast the return value of malloc(). Thanks to unwind

  2. In your case, with strncpy(), you have supplied n as 2, which is not having any scope to store the terminating null byte. without the trminating null, printf() won't be knowing where to stop. Now, with 3 bytes of memory, you can use strcpy() to copy the string properly

strncpy() will not add the terminating null itself, in case the n is equal to the size of supplied buffer, thus becoming very very unreliable (unlike strcpy()). You need to take care of it programmatically.

check the man page for strncpy() and strcpy() here.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • @user3760729 Good to hear that. If this solved your issue, you can consider "accept"-int this answer by clicking the `tick` on the upper left hand side of my post. – Sourav Ghosh Nov 24 '14 at 10:51
  • [Don't recommend casting `malloc()`'s return value in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Also, `strncpy()` will add termination if possible, but not always which makes it tricky and not recommendable. – unwind Nov 24 '14 at 10:53
  • Also, sizeof(char) is a constant 1. Since sizeof returns the number of characters needed to represent the type. It is 100% redundant. – perh Nov 24 '14 at 11:09
  • @perh yes, right you are. i have made a small change. can you please review it? – Sourav Ghosh Nov 24 '14 at 11:16
  • while sizeof(*x) (or x[0]) is recommended if x is complex, for a char* the multiplication can be left out entirely. It is generally speaking assumed that people know malloc allocates in units as returned from sizeof, and that sizeof return the number of char:s used to store something. – perh Nov 24 '14 at 11:22
  • @perh Sir, i am not sure if i have understood you fully, but `malloc(3*sizeof*buffer);` is more robust in case data type of `buffer` is changed. am I right? – Sourav Ghosh Nov 24 '14 at 11:37
  • Well, in this case, since it is used with strncpy and friends that is not likely to happen. So, it is just unneeded code that seems to indicate that the developer does not know that sizeof(char) is always 1. – perh Dec 03 '14 at 08:05