tags:

views:

296

answers:

8

I'm trying to return an array of char*'s to a function. I've simplified my code to a test case that clones a char array that instead of containing chars holds pointers to those chars.

/*
 * code.c
 */
#include <stdio.h>

char* makePointerCopy(char cIn[]);

int main() {
    char cTest[] = {'c', 't', 's', 't'};
    char* cPTest[] = makePointerCopy(cTest);
    printf("%p %c", cPTest, *cPTest);
    fflush(stdout);
    return 0;
}

char* makePointerCopy(char cIn[]) {
    char* cOut[sizeof(cIn)/sizeof(cIn[0])];
    int iCntr;

    for (iCntr = 0; iCntr < sizeof(cIn)/sizeof(cIn[0]); iCntr++)
        cOut[iCntr] = cIn + iCntr;

    return cOut;
}

A couple of warnings aside, this is what the compiler has to say about this code snippet:

invalid initializer (at char* cPTest[] = makePointerCopy(cTest);)

Why does this happen?

+3  A: 

Because makePointerCopy returns a char*, not a char*[].

You should be able to change that line to:

char* cPTest = makePointerCopy(cTest);

More specifically, the reason that you get THAT error message, rather than something about types, is that array initializers are required to be compile-time constants.

From http://bytes.com/topic/c/answers/215573-invalid-initializer

Even if the declaration is not at file scope, it would be illegal in both C90 and C99. C90 demands compile-time constant initializers for automatic and register arrays. And both C90 and C99 require a character array to be initialized with a) a string literal, or b) a brace-enclosed initializer list.

Still, the type mismatch is the actual problem here.

danben
+2  A: 

You need to return a char ** or a char *[].

In particular if you want makePointerCopy to return "an array of char* then you need to actually return such an array. Right now you're returning a pointer to a char, or "char*".

The line of code in question is attempting to assign the result of makePointerCopy, which returns a char* to a char*[]. While this is technically okay in C, and the compiler will still produce a result, the compiler is basically informing you that what it produces may not in fact execute as you expect.

Mark E
+3  A: 

At that line, you are trying to assign a char* to a char* array. Simply put, they are different types.

Aaron
So I need to change the return type? What do I change it to? I tried `char*[]` but that generates loads of errors and warnings.
Pieter
Pieter - see my answer for an example of what to change.
danben
char** should do it, if I remember correctly. Change the type of cPTest to char**, too.
Aaron
+2  A: 

Because your function returns char* while you're assigning it to char*[]. C might have a pretty weak type system but some things shouldn't be done :-)

Joey
A: 

The function returns a char* not a char[]*.

Willis Blackburn
A: 

char * is a pointer to a single char where as you need char ** which is a pointer to an array or char *

Howard May
+2  A: 

Let's start with the warning. You declared:

char* cPTest[]

in English: "cPTest is an array of pointers to char"

and

char* makePointerCopy(char cIn[]);

in English: "makePointerCopy() takes an array of chars and returns a pointer to char"

So you are trying to assign a "pointer to char" to "an array of pointers to chars". Can you see the problem? I would suggest to carefully check the types before doing assignments.

That said, what you really want is to declare makePointerCopy() to return a "pointer to a pointer to char":

char **makePointerCopy(char cIn[]);

because, in the end, you will return the pointer to the first element of the return array.

Another important point: you declared you "cOut" as local variable to a function.

char* makePointerCopy(char cIn[]) {
    char* cOut[sizeof(cIn)/sizeof(cIn[0])];

    ...  /* cOut can ONLY be used within the function! */

    return cOut;  // <-- The address returned point to a 
                  //     block of memory that is no longer valid
                  //     after the end of the function
}

Remember that local variables are automatically made invalid once the function terminates. To "preserve" it you can declare it static:

char* makePointerCopy(char cIn[]) {
    static char* cOut[sizeof(cIn)/sizeof(cIn[0])];

    ...  /* cOut will survive the end of the function */

    return cOut;  // <-- The address can be returned 
}

Note that you must be well disciplined when return such type of values.

As an alternative you can allocate the space you need with malloc() as long as you will remember to free() it when you don't need it anymore.

Remo.D
I think I'm starting to get it. As for the local variable: cOut points to a variable that was not made inside `makePointerCopy`, therefore I should be safe, right? In any event, is there a way to keep C from erasing specific local variable?
Pieter
You're not safe as the cOut *is* declared within the function and will be invalid to use it after you return. You can declare it as "static" to be allowed to return its address. I'm going to edit the answer to show this.
Remo.D
+1  A: 

All of the other answers are correct, but there seem to be quite a lot of other problems with your code:

char* cOut[sizeof(cIn)/sizeof(cIn[0])];

I believe you think sizeof(cIn) returns the amount of memory occupied by the elements in the cIn array. That is incorrect. In this case, sizeof(cIn) will return the size of a pointer on your system, usually 4 or 8 bytes. sizeof(cIn[0]) will return the size of a character, which is 1 byte. There is generally no way to discover the size of an array in C, so I'm afraid you'll have to pass that size to your function.

Also keep in mind that makePointerCopy returns a pointer to a statically allocated block of memory. That memory basically is a local variable of makePointerCopy, and will be released when makePointerCopy has finished doing it's job. In other words, makePointerCopy will return a pointer to invalid memory.

Michilus