CODEWITHSUNDEEP
cgcc
Rogue
Rogue

Reputation: 11483

Attempting to concatenate strings produces a segmentation fault

I'm attempting to convert command-line arguments into a single char* or "c-string" reference, however it seems my code is breaking. There aren't any compiler warnings, so I'm a bit stuck at this point

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

int main(int argc, char** argv) {
    if (argc < 2) {
        printf("usage: program <arguments>");
        return 0;
    }
    char* string = "";
    for (int i = 1; i < argc; i++) {
        strcat(string, argv[i]);
    }
    printf("%s", string);
    printf("\n");
    return 0;
}

Compiling with:

gcc program.c -Wall -std=c99 -o prog

Doesn't seem to throw any warnings, so where could I be going wrong?

Edit:

Updates code to this point:

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

int main(int argc, char** argv) {
    if (argc == 0) {
        printf("usage: program <arguments>");
        return 1;
    }
    int tot = 0;
    for (int i = 1; i < argc; i++) {
        tot += strlen(argv[i]);
    }
    char string[tot + argc]; // total word length
    strcat(string, "\0");
    for (int i = 1; i < argc; i++) {
        strcat(string, argv[i]);
        strcat(string, " ");
    }
    strcat(string, "\0");
    printf("%s", string);
    printf("\n");
    return 0;
}

However a new problem arises, in that it appears to prepend pseudo-random garbage characters to the string. I added this to the catenation loop to look at the output:

for (int w = 0; w < sizeof(argv[i]); w++) {
    printf("\t%s%c\n", "char value: ", argv[i][w]);
}

And this was the output:

./prog one two three
    char value: o
    char value: n
    char value: e
    char value:
    char value: t
    char value: w
    char value: o
    char value:
    char value: t
    char value: h
    char value: r
    char value: e
Tëñ¿one two three

So my guess is that the issue lays within the argument not having a proceeding \0 value, but not 100% certain of how to proceed. Would creating a new array of char to append to, then catenating that be appropriate?

Upvotes: 1

Views: 2698

Answers (3)

jrr
jrr

Reputation: 1997

You're attempting to write into a literal string. Two problems: 1) literal strings aren't generally placed in writable memory. 2) the string (character array) is of size zero; there's no room to write in to it.

The quick answer is to allocate a buffer on the heap:

char* string = (char*)malloc(256);

or on the stack:

char string[256];

Ideally you'd avoid picking a constant size like 256. One approach would be to loop over argv twice - once to measure how much space you'll need, and then again to perform the copies.

Upvotes: 0

moeCake
moeCake

Reputation: 510

You need to count the total space first: 

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

Then allocate space for final string: 

string = calloc(1, total+1);

+1 is for null terminator ('\0').
Then you can strcat to string, and remeber to free it when you don't need it anymore.

Here is the full working code (without error checking): 

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

int main(int argc, char** argv) {
    int total = 0;
    int i;

    if (argc < 2) {
        printf("usage: program <arguments>");
        return 0;
    }
    char *string;
    for (i = 1; i < argc; i++) {
            total += strlen(argv[i]);
    }
    string = calloc(1, total+1);
    for (i = 1; i < argc; i++) {
            strcat(string, argv[i]);
    }   
    printf("%s", string);
    printf("\n");

    free(string);

    return 0;
}

Upvotes: 1

yongzhy
yongzhy

Reputation: 977

the way you declare string make the memory that string point to can't be modified.

Instead, you can declare string as char array if you know max size needed. And you'd beter do length checking to make sure it's not overrun when you do strcat

char string[1024]; // put the size you needed

Or you can dynamically allocate space. As suggested by @moeCake.

Upvotes: 0

Related Questions