Custom Concatenate in C-CodePudding

I'm trying to write my own concatenate program. What I'm doing is getting two strings as input from argv, creating a third empty character array that holds the length of argv[1] argv[2], and then use two for loops to insert the characters from each argv string into the third string.

My first for loop seems to be working fine buy my second for loop isn't doing anything. Any ideas?

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

int main(int argc, char *argv[])
{
  char *string1 = argv[1];
  char *string2 = argv[2];

  int string1Len = strnlen(string1, 50);
  int string2Len = strnlen(string2, 50);

  char string3[string1Len   string2Len   1];

  for (int i = 0; i <= string1Len; i  )
  {
    string3[i] = string1[i];
  }

  for(int i = (string1Len   1); i <= (string1Len   string2Len); i  )
  {
    string3[i] = string2[i];
  }

  string3[string1Len   string2Len   1] = '\0';

  printf("%s %d %d\n", string3, string1Len, string2Len);

  return 0;
}

CodePudding user response：

Your second for loop "did nothing" because the first one worked up to the \0 character and included it in string3, so it's better to set the condition that the for loop works up to that character

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

int main(int argc, char *argv[])
{
  char *string1 = argv[1];
  char *string2 = argv[2];

  int string1Len = strlen(string1);
  int string2Len = strlen(string2);

  int i;

  char string3[string1Len   string2Len  1];

  for (i = 0; string1[i]!='\0'; i  )
  {
    string3[i] = string1[i];
  }
  string3[i]=' ';   //with space
    i;
  for(int j = 0; string2[j]!='\0'; j  )
  {
    string3[i] = string2[j];
    i  ;
  }

  string3[string1Len   string2Len   1] = '\0';

  printf("%s %d %d\n", string3, string1Len, string2Len);

  return 0;
}

CodePudding user response：

The index values in both loops are incorrect:

you should stop the first loop when i == string1Len, hence the test should be:
```
 for (int i = 0; i < string1Len; i  )
```
you should use add string1Len to the index into the destination string so bytes from the second string are appended to those of the first string:
```
for (int i = 0; i < string2Len; i  ) {
    string3[string1Len   i] = string2[i];
}
```
the index for the null terminator is string1Len string2Len, adding 1 is incorrect as indexing is zero based in C:
```
 string3[string1Len   string2Len] = '\0';
```
you should test the actual number of arguments provided to the program to avoid undefined behavior if some are missing.

Here is a modified version:

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

int main(int argc, char *argv[]) {
    if (argc < 3) {
        printf("missing arguments\n");
        return 1;
    }
    char *string1 = argv[1];
    char *string2 = argv[2];

    int string1Len = strnlen(string1, 50);
    int string2Len = strnlen(string2, 50);

    char string3[string1Len   string2Len   1];

    for (int i = 0; i < string1Len; i  ) {
        string3[i] = string1[i];
    }

    for (int i = 0; i < string2Len; i  ) {
        string3[string1Len   i] = string2[i];
    }

    string3[string1Len   string2Len] = '\0';

    printf("%s %d %d\n", string3, string1Len, string2Len);

    return 0;
}

CodePudding user response：

There are two main issues in your code. Your first for loop copies the nul terminator from string1; so, anything you then add to your string3 after that will simply be ignored by functions like printf, because they see that nul as marking the end of the string.

In your second for loop, you have the same problem and, more critically, the i index you use is not valid for string2, as you have added the length of string1 to it.

Also, note that arrays in C start at zero, so you shouldn't add the 1 to the position of the final nul terminator.

Here's the "quick fix" for your current code:

    for (int i = 0; i < string1Len; i  ) { // Use "<" in place of "<=" or we copy the null terminator
        string3[i] = string1[i];
    }
    for (int i = 0; i < string2Len; i  ) { // Start "i" at 0 for valid "string2" index ...
        string3[i   string1Len] = string2[i]; // ... and add "string1Len" for the destination index
    }
    string3[string1Len   string2Len] = '\0'; // Arrays start at ZERO, so don't add 1 for "nul" terminator position

However, there are some other points and possible improvements. Note that the strnlen function returns a size_t type, so you would be better off using that for your indexes. Also, as you know that the i index at the end of the first loop will still be valid for the next character, you can re-use that in the second loop (so long as you have declared it outside the first loop), and you can use a second index for the source string.

Also, as pointed out by chqrlie, you really should check that you have sufficient source data in the argv array.

Here's a version of your program with those additional changes:

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

int main(int argc, char* argv[])
{
    if (argc < 3) {
        // Error-handling code
        return 1;
    }
    char* string1 = argv[1];
    char* string2 = argv[2];
    size_t string1Len = strnlen(string1, 50);
    size_t string2Len = strnlen(string2, 50);
    size_t i, j;
    char string3[string1Len   string2Len   1];

    for (i = 0; i < string1Len; i  ) {
        string3[i] = string1[i];
    }
    for (j = 0; j < string2Len; j  , i  ) {
        string3[i] = string2[j];
    }
    string3[i] = '\0';
    printf("%s %zu %zu\n", string3, string1Len, string2Len); // "%zu" for "size_t"
    return 0;
}