Problem with C function of type char pointer, can someone explain?

Question

Can someone help me find the errors in this C function?

    char* f(int i) {
            int i; 
            char buffer[20];
            switch ( i ) {
                    1: strcpy( buffer, "string1");
                    2: strcpy( buffer, "string2");
                    3: strcpy( buffer, "string3");
                    default: 
                    strcpy(buffer, "defaultstring");
            }
            return buffer;
    }

I think it has to do with type conversion. My compiler gives a warning that 'the declaration of int i shadows a parameter'.

Answer 1

  char* f(int i) {
            int i; 

Error 1: the local 'i' parameter shadows the 'i' argument to the function

        char buffer[20];
        switch ( i ) {

Error 2: You use the local 'i' variable, which is not initialized.

                1: strcpy( buffer, "string1");
                2: strcpy( buffer, "string2");
                3: strcpy( buffer, "string3");
                default: 
                strcpy(buffer, "defaultstring");
        }
        return buffer;

Error 3: You return a pointer to an element in a local array, that pointer is not valid when the function returns. 'buffer' has gone out of scope.

Answer 2

Well, for one thing, you're missing the 'case' declaration in your switch statement as well as the breaks at the end of the statement. Try changing your code to this:

    char* f(int i) {
            int i; 
            char buffer[20];
            switch ( i ) {
                    case 1: strcpy( buffer, "string1"); break;
                    case 2: strcpy( buffer, "string2"); break;
                    case 3: strcpy( buffer, "string3"); break;
                    default: 
                    strcpy(buffer, "defaultstring"); break;
            }
            return buffer;
    }

Also, you're overriding your parameter i with a local variable named i. So remove that:

    char* f(int i) {
            char buffer[20];
            switch ( i ) {
                    case 1: 
                         strcpy( buffer, "string1"); 
                         break;
                    case 2: 
                         strcpy( buffer, "string2"); 
                         break;
                    case 3: 
                         strcpy( buffer, "string3"); 
                         break;
                    default: 
                         strcpy(buffer, "defaultstring"); 
                         break;
            }
            return buffer;
    }

Finally, you are returning a char pointer that points to a local statically declared character array named buffer. buffer falls out of scope as soon as the pointer to its first element is returned (what your return statement does). What that means is you end up with a pointer to a variable that no longer exists. To fix this, you can pass a pointer to buffer into the function, like this:

    void f(char *buffer, int i) {
            switch ( i ) {
                    case 1: 
                         strcpy( buffer, "string1"); 
                         break;
                    case 2: 
                         strcpy( buffer, "string2"); 
                         break;
                    case 3: 
                         strcpy( buffer, "string3"); 
                         break;
                    default: 
                         strcpy(buffer, "defaultstring"); 
                         break;
            }
    }

That last version should work, provided you make sure that buffer is long enough to accept the string. If you really want to make it safe, take in buffer's length as well and check that against the length of the string you are copying.