strcat problem in C, segmentation fault

Question

Hello everyone! i am new to C and recently i have a problem with a strcat function. I have an array of arguments defined like:

#define MAXARGS 10;

char *argvalues[MAXARGS];

All i want is to concatenate the last non-null element of the array with a null terminator. Here is the piece of my code for that:

while (argvalues[i] != NULL) {
  if (argvalues[i] == NULL){
    strcat(argvalues[i-1], '/0');
    printf("i is: %d\n", i);
    break;
  }
  i++;
}

Do you have any idea why segmentation fault happens and is it actually the right way of using strcat? Any help would be much appreciated!!

Thanks!!!!

Answer 1

As Alcon pointed out, you'll never reach the strcat() with this code. You need to change your loop to something like this:

while (i < MAXARGS) {
   ...
}

Also, passing a null pointer to strcat(). It takes a string, not a character. '\0' is a null character, which will get "promoted" to a null pointer. "" or "\0" would be an empty string, but that wouldn't add a null either. strcat(0) is not the right thing to use here, because it looks for the null terminator before concatenating. Therefore, you can't use it to add a null terminator!

If you don't already have a null terminator on each string in the array, or otherwise know its length, I don't see how you can find the end to add a null terminator. It seems like sort of a "call me and I'll tell you my phone number" situation.

Answer 2

Your loop is wandering past the end of your array. That's what's causing the segfault. In C there is no guarantee that an uninitialized variable will be NULL, or anything else for that matter. So your loop keeps looping until it tries to access argvalues[11], which doesn't exist. Hence segmentation fault.

while (argvalues[i] != NULL) {
  if (argvalues[i] == NULL){

These two contradict each other. It will never even reach the if statement because the while loop will exit first when it discovers that argvalues[i] == NULL. And if all 10 of your arguments are set then you will attempt to access argvalues[11] which will segfault as previously mentioned.

To properly use strcat you have to have a string buffer that's large enough to accept the string you are concatenating on to the end of it. For example, if you want to add " world" on to a buffer already containing "hello" the hello buffer must be declared to be at least as long as "hello"+" world" plus 1 more character (the '\0' at the end).

// Here's an example.
char buffer[12];
strcpy(buffer, "hello");
strcat(buffer, " world");

If you try to do this, it will fail:

// Buffer isn't big enough to copy into.
char buffer[] = "hello";
strcat(buffer, " world");

To figure out exactly what you're doing we'll need a little more description and code. The best place to add the null terminator to your arguments would be when the arguments are first set. Depending on how you're setting your arguments it may already be happening for you.

Answer 3

This is not the good way to do that!

If you are sure that your char* has no "\0" you could do your stuff like this:

char* oldargvalues = argvalues;
while(*argvalues++);
argvalues = 0;

(oldargvalues is to keep a track of the beginning of the char*). But even though, it's strange since the loop will end if you have already an "\0" char.

Answer 4

What Alcon and Fred said are true, plus it looks like your argvalues are already null-terminated, so it's not clear what you're really trying to do. You shouldn't use strcat unless the first argument string is in a buffer that you've allocated so you know it's big enough. Like if you have void main(char* argv[], int argc) you would never say strcat(argv[i], ...), even though strcat(..., "") is a no-op.

Answer 5

I don't understand the loop you have. It looks like it will never do anything. In the while loop you check if argvalues[i] != NULL. In the very next statement, you check if it is NULL. One of those is always going to be false (it can't be both NULL and non-NULL), so this loop will never do anything. I think you want something like the following:

#define MAXARGS 10;
char *argvalues[MAXARGS];
int i = 1;
while (i < MAXARGS) 
{
   if (argvalues[i] == NULL)
   {
      //Notice here:  as mentioned by other posters, you need to pass 
      //a string to strcat, not a char
      strcat(argvalues[i-1], "");
      printf("i is: %d\n", i);
      break;
    }

    i++;
}

Answer 6

strcat concatenates two char pointers like so.

char *p1[12];
char *p2 = "world";
strcpy(p1,"hello ");
strcat(p1,p2);

pi now holds - hello world You have to make sure that the destination has enough room for the concatenated result.

Answer 7

Couple of problems:

Your argvalues array is not automatically initialized to be all NULL, you'll have to do that by hand, so right of the bat, you can't detect the NULL in your while() loop as others have indicated.

How is argvalues being set?

Answer 8

Thank you all for the replies!!!!!

i think i solved the problem, mainly what i have done is to create a new char[size] variable, then copy the last argument in it, put the null terminator in that variable and then copy back to the last argument. works ok but the code is messy. so might do smth with it later on. strcat is for strings only, not for chars, thanks for pointing out all the lacks in my code!

Thanks everyone!!!! much appreciated!