list[idx] = strdup(tok[1] +tok[2]) or initial malloc to size of list -C

Question

I have a code that has an invalid write in valgrind. What I tried to do was strcat the idx I malloc'd. I would like to know what size to strdup() so I can put spaces in between the number of tokens, which are the in the char *argv[].

Here is a small piece of the code

If you are interested in a larger picture of how the tokens are collected form getline you can see the question here https://stackoverflow.com/questions/27394313/getline-loop-for-stdin-has-segmentatoin-fault-after-each-loop-c

// parent will wait for child to exit
id = wait( &status );

if( id < 0 ) {
    perror( "wait" );

} else {

    int idx = 0;
    histL[count-1] =  strdup(argv[idx]);
    //printf("Token added %s size of %i is\n", argv[idx], i);
    for (idx = 1; idx < i-1; idx++) {       //-1 because no need last one
        strcat( histL[count-1], " ");
        strcat( histL[count-1], argv[idx]);
        //printf("Token added %s\n", argv[idx]);
    }
    //printf("Exit for Loop\n");
//puts( "Parent is now exiting." );

//exit( EXIT_SUCCESS );
    }   

Here is the best that I can do at this point

int idx = 0;
histL[count-1] =  ( char* )malloc( sizeof( argv ) + (i*sizeof(" ")));// histLSize );    //strdup(argv[idx]);
strcpy(histL[count-1], strdup(argv[idx]));
//printf("Token added %s size of %i is\n", argv[idx], i);
for (idx = 1; idx < i-1; idx++) {       //-1 because no need last one
    histL[count-1] = strcat( histL[count-1], " ");
    histL[count-1] = strcat( histL[count-1], argv[idx]);
    //printf("Token added %s\n", argv[idx]);
}

Fixed it

int idx = 0;
histL[count-1] =  ( char* )malloc( sizeof( &argv ) + (i*sizeof(" ")));// histLSize );   //strdup(argv[idx]);
strcpy(histL[count-1], argv[idx]);
//printf("Token added %s size of %i is\n", argv[idx], i);
for (idx = 1; idx < i-1; idx++) {       //-1 because no need last one
    histL[count-1] = strcat( histL[count-1], " ");
    histL[count-1] = strcat( histL[count-1], argv[idx]);
    //printf("Token added %s\n", argv[idx]);
}

Answer 1

You are not calculating the malloc() size correctly at all. You need to use two loops, one to calculate the size, then another to copy the strings after allocating the memory:

int lastidx = i-1;

int size = strlen(argv[0]) + 1;
for (int idx = 1; idx < lastidx; ++idx) {
    size += (1 + strlen(argv[idx]));
}

histL[count-1] = (char*) malloc(size);

strcpy(histL[count-1], argv[0]);
for (int idx = 1; idx < lastidx; ++idx) {
    strcat(histL[count-1], " ");
    strcat(histL[count-1], argv[idx]);
}

Or better:

int lastidx = i-1;

int size = strlen(argv[0]) + 1;
for (int idx = 1; idx < lastidx; ++idx) {
    size += (1 + strlen(argv[idx]));
}

char *str = (char*) malloc(size);

int len = strlen(argv[0]);
memcpy(str, argv[0], len);
size = len;

for (int idx = 1; idx < lastidx; ++idx) {
    str[size++] = ' ';
    len = strlen(argv[idx]);
    memcpy(str+size, argv[idx], len);
    size += len;
}

str[size] = '\0';
histL[count-1] = str;