C strings need to be null terminated. Ie they need a '\0' character at the end, so common string expressions can tell where the string ends. String functions like strcpy and printf("%s... look for this null termination in their operation. When you do this:

for(i=0; s1[i] != '\0'; i++)
        s2[i] = s1[i];

You fail to copy the null terminator. This means when you print the string, printf will go through the string until it finds the first '\0' character somewhere after the end of your string. This is why you see the extra characters, and in the case where the NULL is outside your strings allotted memory you will actually cause undefined behaviour by accessing unallocated space, which is very very bad.

You either need to null terminate your string by adding:

s2[i] = '\0'

After your for loop. Or even better, use a standard c function strcpy.

Your program has two bugs:

First

Your function CopyAString does not write a '\0' character to the end of the string. This means that the string s2[] does not have a '\0' character and you would not be able to pass s2[] to functions like printf() or other functions that expect an "input" string to end with '\0'.

However, in your program, this is not a problem because the for loop expects a fixed-length string.

Second

In your program, the following problem is more important:

You pass &str as first argument to CopyAString instead of str.

This means that s1 (the first argument of CopyAString) does not point to the character 'H' of "Hello world" but it points to the first byte of the value stored in the variable str...

Note that the variable str is a pointer: It does not store a "value" (here: the string "Hello world") but it stores an address of a value!

If the string "Hello world" is stored in the RAM at address 0x20 44 00 40 (this means: 0x40004420 on an x86 or ARM computer or 0x20440040 on a PowerPC), the variable str will contain the value 0x20 44 00 40.

s1[0] will be 0x20 (which is the space character). s1[1] will be 0x44 (which is 'D')...

Good interview question has several layers, to which to candidate can demonstrate different levels of understanding.

On the syntactic 'C language' layer, the following code is from the classic Kernighan and Ritchie book ('The C programming language'):

while( *dest++ = *src++ )
    ;

In an interview, you could indeed point out the function isn't safe, most notably the buffer on *dest isn't large enough. Also, there may be overlap, i.e. if dest points to the middle of the src buffer, you'll have endless loop (which will eventually creates memory access fault).

There are multiple problems in your code:

• The loop test in the length function is incorrect: instead of i < str[i], it should be:

   for (i = 0; str[i] != '\0'; i++)
  

• the same problem in the concat function. Change the loop to:

   for (j = 0; src[j] != '\0'; j++) {
  

• also in the concat function, i should be the length of dst, not that of src. You might use len instead of i for this variable.

• The array str4 in function main does not have any space available at the end for concat to append anything. Define it with a larger size this way:

  char str4[MAXSIZE] = "Plain old string";      
  

Here is the corrected version:

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

#define MAXSIZE 32

void concat(char dest[], char src[]) {
    int len = length(dest);
    int j;
    for (j = 0; src[j] != '\0'; j++) {
        dest[len + j] = src[j];
    }
    dest[len + j] = '\0';
}

int length(char str[]) {
    int len = 0;
    int i;
    for (i = 0; i < str[i]; i++) {
        len++;
    }
    return len;
}

int main(void) {
    // Variable declarations for all parts
    char str2[MAXSIZE] = "Troy";
    char str4[MAXSIZE] = "Plain old string";
    char str6[MAXSIZE];
    // Part 6
    printf("\n----- Part 6 -----\n");
    // Make a copy of the destination string first, to be reused later
    strcpy(str6, str4);
    concat(str4, str2);
    strcat(str6, str2);
    printf("Comparing results of concat and strcat ...\n");
    printf("strcmp(\"%s\", \"%s\") says: %d\n",
           str4, str6, strcmp(str4, str6));
    return 0;
}