Returning an array of pointers to `char`

Question

This program should concatenate strings, But I don't know how return the string array back to main.

char **conca(char *a[], int n)
{

char buff[200];

char **conc[n];
for (int i = 0; i < n; i++)
{
    strcpy(buff,a[i]);
    strcat(buff,"-");
    int l = strlen(buff);
    *conc[i] = malloc((l+1)*sizeof(char));
    strcpy(*conc[i],buff);
}

return *conc;

In main.c:

char **conca(char *a[], int n);

int main(int argc, char *argv[])
{
    if(argc == 1)
    {
        printf("Uso: %s <stringa> <stringa> ... <stringa> \n",argv[0]);
        return 1;
    }

    int dim = argc - 1;

    int pos = 0;
    char *array[dim];

    for(int i = 1; i <= dim; i++ )
    {
        array[pos] = malloc((strlen(argv[i])+1)*sizeof(char));
        strcpy(array[pos],argv[i]);
        pos++;
    }

    char **f = conca(array, dim);
}

The program triggers a segmentation fault (core dump).

How can I print the concatenated string in main?

The segfault happens because you do not have memory for `*conc[i]` when you copy: `*conc[i] = malloc((l+1)*sizeof(char));` If you want to return a string you should only use single pointers. Not pointers to pointers. — Gerhardh, Dec 27 '16 at 09:46
Do you want `string1 string2 ... stringn` to `string1-string2- ... -stringn` ? — BLUEPIXY, Dec 27 '16 at 10:13
Please read [The Definitive C Book Guide and List](http://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list.) before [write in C](https://www.youtube.com/watch?v=1S1fISh-pag). — Stargateur, Dec 27 '16 at 10:32
the expression: `sizeof(char)` is defined in the C standard as 1. Multiplying anything by 1 has no effect. so that expression is just cluttering the code. Suggest removing that expression — user3629249, Dec 28 '16 at 19:23

score 2 · Answer 1 · answered Dec 27 '16 at 10:39

You need return char* instead of array of pointer to char.
Like this

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

char *join(char *a[], int n, char sep){
    size_t lens[n], total_length = 0;

    for (int i = 0; i < n; i++){
        total_length += (lens[i] = strlen(a[i]));
    }
    total_length += n;//sep * (n-1) + NUL

    char *ret = malloc(total_length);
    char *wk = ret;

    for (int i = 0; i < n; i++){
        if(i)
            *wk++ = sep;
        memcpy(wk, a[i], lens[i]);
        wk += lens[i];
    }
    *wk = 0;

    return ret;
}

int main(int argc, char *argv[]){
    if(argc == 1){
        printf("Uso: %s <stringa> <stringa> ... <stringa> \n", argv[0]);
        return 1;
    }

    int dim = argc - 1;
    char *concata = join(argv+1, dim, '-');
    puts(concata);
    free(concata);
}

I like the `size_t lens[n]` to allow code to use the fast `memcpy()`. Perhaps make code tolerant of `n == 0` too with separate test? — chux - Reinstate Monica, Sep 25 '17 at 15:14

Sumit Gemini · Answer 2 · 2016-12-27T10:29:55.427

Reason of segfault is you can not initialize memory for *conc[i] like that :

*conc[i] = malloc((l+1)*sizeof(char))

instead you have to do

conc[i] = malloc((l+1)*sizeof(char))

But you have another problem here. You're declaring the array as a local variable. conc is an array of pointers which is stored in conca()'s stack frame, so this is, technically speaking, undefined behavior. The solution is to change conc to a char ** and use malloc() to allocate the whole array (and remember to free it later)

So i modified your char **conca(char *a[], int n) function. so i used **conc instead of array of pointer.

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

char **conca(char *a[], int n)
{

char buff[200];

char **conc = (char **)malloc(n*sizeof(char *));
int i=0;
for (i = 0; i < n; i++)
{
    strcpy(buff,a[i]);
    strcat(buff,"-");

    int l = strlen(buff);
    conc[i]=(char *)malloc((l+1)*sizeof(char));
    strcpy(conc[i],buff);
}

return conc;
}

int main(int argc, char *argv[])
{
        if(argc == 1)
        {
                printf("Uso: %s <stringa> <stringa> ... <stringa> \n",argv[0]);
                return 1;
        }

        int dim = argc - 1;

        int pos = 0;
        char *array[dim];
        int i=0;
        for(i = 1; i <= dim; i++ )
        {
                array[pos] = malloc((strlen(argv[i])+1)*sizeof(char));
                strcpy(array[pos],argv[i]);
                pos++;
                pos++;
        }
        char **f = conca(array, dim);

        for(i=0;i<dim;i++)
        printf("%s",f[i]);

        printf("\n\n");
}

`conc[i] = malloc((l+1)*sizeof(char))` is wrong: you forgot to update the type inside `sizeof`. Please also see [why not to cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — Quentin, Dec 27 '16 at 12:27

RoadRunner · Answer 3 · 2016-12-28T02:13:03.140

0

Instead of returning char **, You should return char *.

There is also no error checking on malloc, which is needed since you can return a NULL pointer if unsuccessful.

Here is an example that shows this:

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

char *join(char *argv[], int n);

int main(int argc, char *argv[]){
    char *result;

    if(argc == 1){
        printf("Uso: %s <stringa> <stringa> ... <stringa> \n", argv[0]);
        return 1;
    }

    result = join(argv, argc);
    printf("%s\n", result);

    free(result);
    result = NULL;

    return 0;
}

char *join(char *argv[], int n) {
    int i;
    const char *sep = "-";
    char *string;
    size_t strsize = 0;

    for (i = 1; i < n; i++) {
        strsize += strlen(argv[i])+1;
    }

    string = malloc(strsize);
    if (!string) {
        printf("Cannot allocate memory for string.\n");
        exit(EXIT_FAILURE);
    }

    *string = '\0';
    for (i = 1; i < n; i++) {
        strcat(string, argv[i]);
        if (i < n-1) {
            strcat(string, sep);
        }
    }
    return string;
}

Input:

$ gcc -Wall -Wextra -Wpedantic -std=c99 -o concat concat.c

$ ./concat Always check return of malloc

Output:

Always-check-return-of-malloc

edited Dec 28 '16 at 02:13

answered Dec 27 '16 at 14:05

RoadRunner

25,803
6
42
75

do we need to extend the "`char *p` or `char* p`" debate to include whether the star goes before or after the newline ?? – M.M Dec 27 '16 at 14:38
@M.M I'm not sure what you mean. is there something wrong with my code? – RoadRunner Dec 27 '16 at 14:48
My code seems to match the second style, is that a bad thing? – RoadRunner Dec 27 '16 at 18:09
It's very unusual to have `char` on one line then `*join` in the next line – M.M Dec 27 '16 at 20:12
@M.M changed it. I don't know why, but I've always done that with function headers. – RoadRunner Dec 28 '16 at 02:15
Use of `strcat()` make `join()` `O(strsize * n)`. By keeping track of the string end, code could be `O(strsize + n)`. – chux - Reinstate Monica Sep 25 '17 at 15:16

user3629249 · Answer 4 · 2016-12-28T19:45:55.740

the following code:

cleanly compiles
performs the desired functionality
properly outputs error messages to stderr
properly checks for errors
includes the proper #include statements
is appropriately commented
contains some the original posted code as comments to highlite where changes were made
avoids unnecessary variables
displays the resulting concatenated string
cleans up after itself

and now the code

#include <stdio.h>  // printf(), fprintf()
#include <stdlib.h> // exit(), EXIT_FAILURE, malloc(), realloc(), free()
#include <string.h> // strlen(), strcat()


//char **conca(char *a[], int n);
// function prototypes
char *concat(char *argv[], int argc);

int main(int argc, char *argv[])
{
    if(argc == 1)
    {
        fprintf( stderr, "Uso: %s <stringa> <stringa> ... <stringa> \n",argv[0]);
        //printf("Uso: %s <stringa> <stringa> ... <stringa> \n",argv[0]);
        exit( EXIT_FAILURE );
        //return 1;
    }

    char *newCat = concat( argv, argc );

    printf( "%s\n", newCat );
    free( newCat );
} // end function: main


char *concat(char *argv[], int argc)
{

    char *newString = malloc(1);
    if( !newString )
    { // then malloc failed
        perror( "malloc failed" );
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful
    newString[0] = '\0';

    for( int i = 1; i <= argc; i++ )
    //for(int i = 1; i <= dim; i++ )
    {
        char * tempptr = realloc( newString, strlen( newString) + strlen(argv[i])+2 );
        if( !tempptr )
        { // then realloc failed
             perror( "realloc failed" );
             free( newString );
             exit( EXIT_FAILURE );
        }

        // implied else, realloc successful

        newString = tempptr;
        //array[pos] = malloc((strlen(argv[i])+1)*sizeof(char));
        //strcpy(array[pos],argv[i]);
        strcat( newString, argv[i] );

        // avoid a trailing '-' in final string
        if( i < (argc-1) )
        { 
            strcat( newString, "-" );
        }
    } // end for

    return newString;
} // end function: concat

Returning an array of pointers to `char`

4 Answers4