struct returning character array printing a value twice

Question

It's a simple program in which function abc is returning an array. But the output is

Thanks
abcdefThanks

Why so? I want Thanks to be printed only once. Also I need to take the size of a as 6. In this program, it doesn't matter but I am doing raw socket programming where I need it. size = 6 is declared in the predefined header files there. How can I implement it?

char *abc()
{
    unsigned char *ch;
    unsigned char a[7],c[6];
    strncpy(a,"Thanks",strlen("Thanks"));
    strncpy(c,"abcdef",strlen("abcdef"));
    ch=malloc(50);
    memset(ch,0,50);
    memcpy(ch,&a,strlen(a));
    memcpy(ch+strlen(a)+1,&c,strlen(c));
    return ch;
}
int main()
{
    char *a;
    a=abc();
    printf("\n%s\n",a);
    printf("\n%s\n",(a+7));
    fflush(stdout);
    return 0;
}

Thanks :)

Is really you got out put like this? because `a` will print `Thanks�abcdef` and `a+7` will `abcdef`. — Jayesh Bhoi, Apr 15 '14 at 07:20

Marco A. · Accepted Answer · 2014-04-15T07:24:46.677

Calling strlen(a) is not stopping where you think it should because there's no zero-terminator and garbage memory is spoiling your result. strlen(string) doesn't include the count of a zero-terminator

You should rather do the following (look at the comments)

char *abc()
{
    char *ch;
    char a[7],c[7];
    strncpy(a,"Thanks",strlen("Thanks")); // Watch out, strlen(string) doesn't include null terminator
    a[6] = '\0'; // Prevent garbage from uninitialized memory to pester your ch and strlen(a)
    strncpy(c,"abcdef",strlen("abcdef"));
    c[6] = '\0';
    ch=malloc(50);
    memset(ch,0,50);
    memcpy(ch,&a,strlen(a));
    memcpy(ch+strlen(a),&c,strlen(c)); // No -1 because you want to cut the terminator off
    return ch;
}
int main()
{
    char *a;
    a=abc();
    printf("\n%s\n",a);
    printf("\n%s\n",(a+7));
    fflush(stdout);
    return 0;
}

The above was compiled with C++ but it should pretty much be the same with a few adjustments.

Here's with a memory-like dumping where # means garbage

char *abc()
{
    char *ch;
    char a[7],c[7];
    strncpy(a,"Thanks",strlen("Thanks")); // Watch out, strlen(string) doesn't include null terminator
    // a = "Thanks##################################.."
    a[6] = '\0'; // Prevent garbage from uninitialized memory to pester your ch and strlen(a)
    // a = "Thanks\0############################"
    strncpy(c,"abcdef",strlen("abcdef"));
    // c = "abcdef############################"
    c[6] = '\0';
    // c = "abcdef\0############################"
    ch=malloc(50);
    // ch = "###############################"
    memset(ch,0,50);
    // ch = "000000000000000000000000000000"
    memcpy(ch,&a,strlen(a));
    // ch = "Thanks000000000000000000000000"
    memcpy(ch+strlen(a),&c,strlen(c)); // No -1 because you want to cut the terminator off
    // ch = "Thanksabcdef00000000000000000"
    return ch;
}

[Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). — unwind, Apr 15 '14 at 07:22
@unwind nice to see your comment. you always keep correcting everyone on this. Well done!! Thanks :) — user3433848, Apr 15 '14 at 07:28
@user3433848 It's a bit of an uphill struggle, but I try. :) — unwind, Apr 15 '14 at 07:31

Basile Starynkevitch · Answer 2 · 2014-04-15T07:19:31.550

0

You forgot to zero terminate your strings by appending a 0 byte.

unsigned char a[7], c[7]; // 7 = 6+1 since "Thanks" and "abcdef" have 6 bytes
strncpy (a, "Thanks", 6);
a[6] = (char)0;
strncpy (c, "abcdef", 6);
c[6] = (char)0;

your c string was too short and not explicitly 0-ended. So your strlen(c) is undefined behavior

edited Apr 15 '14 at 07:19

answered Apr 15 '14 at 07:14

Basile Starynkevitch

223,805
18
296
547

1

`(char) 0` is a rather weird way of writing `'\0'`, in my opinion. – unwind Apr 15 '14 at 07:31

struct returning character array printing a value twice

2 Answers2