Why doesn't strcpy work?

Question

    char sentence2[10];

    strncpy(sentence2, second, sizeof(sentence2));  //shouldn't I specify the sizeof(source) instead of sizeof(destination)?

    sentence2[10] = '\0';                       //Is this okay since strncpy does not provide the null character.

    puts(sentence2);
//////////////////////////////////////////////////////////////

    char *pointer = first;
    for(int i =0; i < 500; i++)                 //Why does it crashes without this meaningless loop?!
    {
        printf("%c", *pointer);
        if(*pointer == '\n')
            putchar('\n');
        pointer++;
    }

So here's the problem. When I run the first part of this code, the program crashes. However, when I add the for loop that just prints garbage values in memory locations, it does not crash but still won't strcpy properly.

Second, when using strncpy, shouldn't I specify the sizeof(source) instead of sizeof(destination) since I'm moving the bytes of the source ?

Third, It makes sense to me to add the the null terminating character after strncpy, since I've read that it doesn't add the null character on its own, but I get a warning that it's a possible out of bounds store from my pelles c IDE.

fourth and most importantly, why doesn't the simply strcpy work ?!?!

////////////////////////////////////////////////////////////////////////////////////

UPDATE:

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

void main3(void)
{
    puts("\n\n-----main3 reporting for duty!------\n");
    char *first = "Metal Gear";
    char *second = "Suikoden";
    printf("strcmp(first, first)  = %d\n", strcmp(first, first));   //returns 0 when both strings are identical.
    printf("strcmp(first, second) = %d\n", strcmp(first, second));  //returns a negative when the first differenet char is less in first string.  (M=77  S=83)
    printf("strcmp(second, first) = %d\n", strcmp(second, first));  //returns a positive when the first different char is greater in first string.(M=77  S=83)

    char sentence1[10]; 
    strcpy(sentence1, first);
    puts(sentence1);
    char sentence2[10];
    strncpy(sentence2, second, 10); //shouldn't I specify the sizeof(source) instead of sizeof(destination).
    sentence2[9] = '\0';                        //Is this okay since strncpy does not provide the null character.

    puts(sentence2);
    char *pointer = first;
    for(int i =0; i < 500; i++)                 //Why does it crashes without this nonsensical loop?!
    {
        printf("%c", *pointer);
        if(*pointer == '\n')
            putchar('\n');
        pointer++;
    }
}

This is how I teach myself to program. I write code and comment all I know about it so that the next time I need to look up something, I just look at my own code in my files. In this one, I'm trying to learn the string library in c.

Answer 1

Your problems start from here:

char sentence1[10]; 
strcpy(sentence1, first);

The number of characters in first, excluding the terminating null character, is 10. The space allocated for sentence1 has to be at least 11 for the program to behave in a predictable way. Since you have already used memory that you are not supposed to use, expecting anything to behave after that is not right.

You can fix this problem by changing

char sentence1[10]; 

to

char sentence1[N]; // where N > 10.

But then, you have to ask yourself. What are you trying to accomplish by allocating memory on the stack that's on the edge of being wrong? Are you trying to learn how things behave at the boundary of being wrong/right? If the answer to the second question is yes, hopefully you learned from it. If not, I hope you learned how to allocate adequate memory.

Answer 2

char *first = "Metal Gear";
char sentence1[10]; 
strcpy(sentence1, first);

This doesn't work because first has 11 characters: the ten in the string, plus the null terminator. So you would need char sentence1[11]; or more.

strncpy(sentence2, second, sizeof(sentence2));  

//shouldn't I specify the sizeof(source) instead of sizeof(destination)?

No. The third argument to strncpy is supposed to be the size of the destination. The strncpy function will always write exactly that many bytes.

If you want to use strncpy you must also put a null terminator on (and there must be enough space for that terminator), unless you are sure that strlen(second) < sizeof sentence2.

Generally speaking, strncpy is almost never a good idea. If you want to put a null-terminated string into a buffer that might be too small, use snprintf.

This is how I teach myself to program.

Learning C by trial and error is not good. The problem is that if you write bad code, you may never know. It might appear to work , and then fail later on. For example it depends on what lies in memory after sentence1 as to whether your strcpy would step on any other variable's toes or not.

Learning from a book is by far and away the best idea. K&R 2 is a decent starting place if you don't have any other.

If you don't have a book, do look up online documentation for standard functions anyway. You could have learnt all this about strcpy and strncpy by reading their man pages, or their definitions in a C standard draft, etc.

Answer 3

this is an array bounds write error. The indices are only 0-9

sentence2[10] = '\0';

it should be

sentence2[9] = '\0';

second, you're protecting the destination from buffer overflow, so specifying its size is appropriate.

EDIT:

Lastly, in this amazingly bad piece of code, which really isn't worth mentioning, is relevant to neither strcpy() nor strncpy(), yet seems to have earned me the disfavor of @nonsensicke, who seems to write very verbose and thoughtful posts... there are the following:

char *pointer = first;
for(int i =0; i < 500; i++)
{
    printf("%c", *pointer);
    if(*pointer == '\n')
        putchar('\n');
    pointer++;
}

Your use of int i=0 in the for loop is C99 specific. Depending on your compiler and compiler arguments, it can result in a compilation error.

for(int i =0; i < 500; i++)

better

int i = 0;
...
for(i=0;i<500;i++)

You neglect to check the return code of printf or indicate that you are deliberately ignoring it. I/O can fail after all...

printf("%c", *pointer);

better

int n = 0;
...
n = printf("%c", *pointer);
if(n!=1) { // error! }

or

(void) printf("%c", *pointer);

some folks will get onto you for not using {} with your if statements

if(*pointer == '\n') putchar('\n');

better

if(*pointer == '\n') {
    putchar('\n');
}

but wait there's more... you didn't check the return code of putchar()... dang

better

unsigned char c = 0x00;
...
if(*pointer == '\n') {
    c = putchar('\n');
    if(c!=*pointer) // error
}

and lastly, with this nasty little loop you're basically romping through memory like a Kiwi in a Tulip field and lucky if you hit a newline. Depending on the OS (if you even have an OS), you might actually encounter some type of fault, e.g. outside your process space, maybe outside addressable RAM, etc. There's just not enough info provided to say actually, but it could happen.

My recommendation, beyond the absurdity of actually performing some type of detailed analysis on the rest of that code, would be to just remove it altogether.

Cheers!