concatenating all strings in argv[] together

Question

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

int main(int argc, string argv[])
{
    string destination = argv[1];

    for (int i = 1; i < argc; i++)
    {
        strcat(argv[i], argv[i+1]);
    }
    printf("%s\n", destination);
}

I need all strings in argv[ ] to be concatenated together. Following program works but at the end give seg fault (core dumped). How can I avoid that?

Answer 1

Neither argv[i] has enough space to be able to store an appended string (of course if the appended string is not an empty string).

Moreover argv[argc] is a null pointer. As a result in the for loop in this call

strcat(argv[i], argv[i+1]);

there is used null pointer argv[i+1] when i is equal to argc-1 that again results in undefined behavior.

From the C Standard (5.1.2.2.1 Program startup)

argv[argc] shall be a null pointer

At least you should write

strcat(destination, argv[i]);

instead of

strcat(argv[i], argv[i+1]);

provided that the array destination can contain all the concatenated strings.

You need to allocate dynamically a large enough array that can be able to store all the concatenated strings.

For example

size_t length = 0;

for ( int i = 1; i < argc; i++ )
{
    length += strlen( argv[i] );
}

char *s = malloc( length + 1 );

if ( s )
{
    *s = '\0';

    for ( int i = 1; i < argc; i++ )
    {
        strcat( s, argv[i] );
    }

    puts( s );
}

free( s );

Or as you are using the type alias string for the type char * then instead of

char *s = malloc( length + 1 );

you may write

 string s = malloc( length + 1 );

Answer 2

CS50 considered harmful. There is no string class in C, it is a low-level language compared to most other languages. There is no automatic memory management. You have to roll out everything manually.

argv[n] happens to be a location where you are allowed to write, but doing so is bad practice and there's only enough room in that location to store the argument string already passed, so you can't append anything there.

The algorithm you need to use goes as:

• Sum all the string lengths of the passed arguments. For example:

  size_t argv_strlen [argc];
  size_t total_size=0;
  for (size_t i=1; i<argc; i++)
  {
    argv_strlen[i] = strlen(argv[i]);
    total_size += argv_strlen[i];
  }
  total_size += 1; // make room for null termination
  

• Allocate enough memory to hold the concatenated strings. For example:

  #include <stdlib.h>
  ...
  char* new_str = malloc (total_size);
  if(new_str == NULL)
  { /* no memory available, handle error */ }
  

• Copy the strings one by one into the allocated space. This can be done in several ways.

  The naive but beginner friendly version is to use strcat (string.h). It concatenates strings. In order to use it, the first item used must be a null terminated string, so it goes like this:

  // make sure the first item is a null terminator:
  *new_str = '\0';
  /* new_str is now an empty string but with sufficient 
     space available behind the null terminator */
  
  for(size_t i=1; i<argc; i++)
  {
    strcat(new_str, argv[i]);
  }
  

  This is however very slow since strcat has to search for the null terminator over and over. The more professional version is to save the string lengths while we grabbed them earlier (aha so that's what the argv_strlen array was for), then copy that much memory + 1 byte for the null terminator using memcpy (string.h) which is much faster than strcat.

  char* copy_here = new_str; // temporary pointer to keep track of where to copy
  for(size_t i=1; i<argc; i++)
  {
    memcpy(copy_here, argv[i], argv_strlen[i]+1);
  
    // point at the next null terminator, then overwrite it the next iteration:
    copy_here += argv_strlen[i]; 
  }
  

Complete example:

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

int main(int argc, char* argv[])
{
  if(argc <= 1)
  {
    // print some error message here
    return 0;
  }
  
  size_t argv_strlen [argc];
  size_t total_size=0;
  for (size_t i=1; i<argc; i++)
  {
    argv_strlen[i] = strlen(argv[i]);
    total_size += argv_strlen[i];
  }
  total_size += 1; // make room for null termination

  char* new_str = malloc (total_size);
  if(new_str == NULL)
  {
    // print some error message here
    return 0;
  }

  char* copy_here = new_str;
  for(size_t i=1; i<argc; i++)
  {
    memcpy(copy_here, argv[i], argv_strlen[i]+1);
    copy_here += argv_strlen[i];
  }

  puts(new_str);
  free(new_str);
}

Command line input progname hello world how are you? gives helloworldhowareyou?