0
votes

Why does this strncpy() implementation crashes on second run, while the first run works ok?

strncpy

Copy characters from string Copies the first n characters of source to destination. If the end of the source C string (which is signaled by a null-character) is found before n characters have been copied, destination is padded with zeros until a total of n characters have been written to it.

No null-character is implicitly appended at the end of destination if source is longer than n (thus, in this case, destination may not be a null terminated C string).

char *strncpy(char *src, char *destStr, int n)
{
    char *save = destStr; //backing up the pointer to the first destStr char
    char *strToCopy = src; //keeps [src] unmodified

    while (n > 0)
    {
        //if [n] > [strToCopy] length (reaches [strToCopy] end),
        //adds n null-teminations to [destStr]
        if (strToCopy = '\0') 
            for (; n > 0 ; ++destStr)
                *destStr = '\0';

        *destStr = *strToCopy;
        strToCopy++;
        destStr++;
        n--;

        //stops copying when reaches [dest] end (overflow protection)
        if (*destStr == '\0')
            n = 0; //exits loop
    }

    return save;
}

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

int main()
{
    char st1[] = "ABC";
    char *st2;
    char *st3 = "ZZZZZ";
    st2 = (char *)malloc(5 * sizeof(char));


    printf("Should be: ZZZZZ\n");
    st3 = strncpy(st1, st3, 0);
    printf("%s\n", st3);

    printf("Should be: ABZZZZZ\n");
    st3 = strncpy(st1, st3, 2);
    printf("%s\n", st3);

    printf("Should be: ABCZZZZZ\n");
    st3 = strncpy(st1, st3, 3);
    printf("%s\n", st3);

    printf("Should be: ABC\n");
    st3 = strncpy(st1, st3, 4);
    printf("%s\n", st3);

    printf("Should be: AB\n");
    st2 = strncpy(st1, st2, 2);
    printf("%s\n", st2);

    printf("Should be: AB\n");
    st2 = strncpy(st1, st2, 4);
    printf("%s\n", st2);
}
3
Does this even compile? strnacpy is undefined.netcoder
Please don't put the question in the title only, put it in the body of the actual question together with some descriptive text.Some programmer dude
Because you run out of allocated memory. First try is to reserve more space for st2 stringGeorge Gaál
Every time I post a question I hope I didn't make a stupid mistake. I totally meant if (strToCopy == '\0') (I did leave a comment though)tempy
@GeorgeGaál strncpy()'s "feature" of tail-filling the target buffer with 0's out to 'n' on an under-copy is probably second only to the failure to place a 0 on exact-copy or over-copy in terms of "didn't know it did that" factor. It still amazes me how many people use that function without knowing either of those features.WhozCraig

3 Answers

5
votes

You get a segmentation fault because

char *st3 = "ZZZZZ";

the destination is a string literal. String literals must not be modified, and often they are stored in write-protected memory. So when you call

strncpy(st1, st3, n);

with an n > 0, you are trying to modify the string literal and that results in a crash (not necessarily, but usually).

In the copy loop, you have forgotten to dereference strToCopy

if (strToCopy = '\0')

and wrote = instead of ==, so strToCopy is set to NULL, causing further dereferences of strToCopy to invoke undefined behaviour.

2
votes

I don't think you want this:

if (strToCopy = '\0') 

Instead, what you probably meant to do is this:

if (*strToCopy == '\0') 

In general, using yoda conditions will save you much headache from comparison-vs-assignment conflation issues:

if ('\0' == *strToCopy)
0
votes
while (n > 0)
    {
        //if [n] > [strToCopy] length (and reaches [strToCopy] end),
        //adds n null-teminations to [destStr]

        *destStr = *strToCopy;
        //stops copying when reaches [dest] end (overflow protection)
        if (*destStr == '\0')
            break; //exits loop
        strToCopy++;
        destStr++;
        n--;


    }
if (*destStr != '\0') *destStr = '\0';

In the main:

printf("Should be: ZZZZZ\n");
    st3 = strncpy(st1, st3, 0);
    printf("%s\n", st3);

this wrong because the length you want to copy is 0 so you will not obtain ZZZZZ