C: problem with char*

Question

/*
 * code.c
 *
 * TASK
 *      Reverse a string by reversing pointers. Function should use return
 *      type char* and use a char* parameter as input.
 */
#include <stdio.h>
#include <string.h>
#define STRMAX 51

char* reverse(char* sPhrase[]);

int main() {
    char sPhrase[STRMAX];
    char sReverse[STRMAX];
    printf("Enter string (max. 50 chars): ");
    gets(sPhrase);
    sReverse = reverse(sPhrase);

    return 0;
}

char* reverse(char* sPhrase[]) {
    char* sOutput[STRMAX];
    int iCnt = 0, iCntRev;

    for (iCntRev = strlen(*sPhrase)-2; iCntRev >= 0; iCntRev--) {
        sOutput[iCnt] = sPhrase[iCntRev];
        iCnt++;
    }

    *sOutput[iCnt] = '\0';      // Don't forget to close the string

    return sOutput;
}

This code has some quirks:

• sReverse = reverse(sPhrase);

  • [Error] incompatible types in assignment
  • [Warning] passing arg 1 of `reverse' from incompatible pointer type

• return sOutput;

  • [Warning] function returns address of local variable
  • [Warning] return from incompatible pointer type

What do these warnings mean? How can I patch the errors? The function should keep char* as a return type and as a parameter since I'm making this little program as part of a C training course.

Answer 1

I see a few problems. First of all, char* sOutput[STRMAX] is an array of char*'s - presumably you meant char sOutput[STRMAX]?

Secondly, and more importantly, when you declare an array in a function in that way (char sOutput[STRMAX]), it gets allocated on the stack and deallocated when the function returns. Thus, if you try to return it, you'll get undefined results, because it's not technically supposed to exist anymore!

The solution is to pass a buffer into the function for it to use:

char* reverse(char const sPhrase[], char sOutput[])

(I added the const so you don't accidentally overwrite sPhrase). Then call reverse like this:

reverse(sPhrase, sReverse);

Now as for whether your algorithm works...

Answer 2

Getting a couple things out of the way:

1. NEVER NEVER NEVER use gets(); it will introduce a point of failure in your program. If you pass gets a buffer sized to hold 10 characters, and the user types in 100 characters, gets() will happily write those extra 90 characters to the memory immediately following your buffer, leading to all kinds of mayhem. Buffer overruns are an easy and common malware exploit, and any code that uses gets() is insecure by design. Use fgets() instead.

2. You cannot assign array objects as you do in the line sReverse = reverse(sPhrase);. If you want to copy the contents of the string being returned by reverse, you need to use strcpy() or similar: strcpy(sReverse, reverse(sPhrase));

The "incompatible types in assignment" diagnostic comes from the fact that you are trying to assign a pointer value (char *) to an array object (char [STRMAX]). As I mentioned above, you cannot assign to an array object anyway.

The "passing argument from incompatible type" warning comes from the fact that your function definition is not typed to expect a pointer to char, but a pointer to pointer to char. Change the function definition to

char *reverse(char *sPhrase) {...}

Why *sPhrase instead of sPhrase[] (or *sPhrase[])? First of all, when an array expression appears in most contexts, its type is implicitly converted from "N-element array of T" to "pointer to T" (the expression decays to a pointer type) and its value is set to the address of the first element in the array; the exceptions to this rule are when the array expression is the operand of either the sizeof or & (address-of) operators, or if the array expression is a string literal being used to initialize an array of char in a declaration. Second of all, in the context of a function parameter definition, T a[] is identical to T *a; both forms declare a as a pointer to T.

When you call reverse(sPPhrase); in main, the type of the expression sPPhrase decays from "STRMAX-element array of char" to "pointer to char", so the formal parameter in the function needs to be typed as char *.

You can still apply the subscript operator to sPhrase, since subscripting is defined in terms of pointer arithmetic, but remember that sPhrase is a pointer value, not an array.

As you currently have it written, reverse expects a parameter of type char *[], which is identical to char **, as though you were passing an array of pointer to char, rather than an array of char. Similarly, your sOutput variable in reverse needs to be declared char sOutput[STRMAX];, not char *sOutput[STRMAX]; (the latter declares sOutput as an array of pointer to char, which is not what you want here; this is the source of the "return from incompatible types" warning as well).

The "return address of local variable" warning comes from the fact that you are trying to return the address of a variable that's local to the function and has automatic extent. Once the function exits, that variable no longer exists, and the value stored at that location may no longer be valid. There are several ways around this:

1. Declare sOutput as static: static char sOutput[STRMAX];. This will cause the memory for sOutput to be allocated at program startup and stay allocated until the program exits, so the contents of the array will persist between calls to reverse. The variable is still local to the function (it cannot be accessed by name outside of that function). However, this means the function is no longer thread-safe, and it's an ugly solution.

2. Dynamically allocate a buffer within reverse and return the address of that. The virtue of this is that you can size the buffer as needed, and you don't have to worry about thread safety. The buffer will persist until you explicitly deallocate it (or until the program exits). The disadvantage is that the caller is now responsible for deallocating that memory when it's finished with it. The best way to avoid headaches with memory management is to avoid memory management in the first place, and this problem doesn't really call for it.

3. Perform the reverse in place (that is, do the reverse within the input array), and return the address of the input array. This means you're clobbering your input, which may not be what you want.

4. Pass the destination array as a second input to the function and don't bother with a return value (or, if you have to return something, return the address of the destination array):

    char *reverse(char *src, char *dst)
    {
      // write contents of src in reverse order to dst
      return dst;
    }
    ...
    reverse(sPPhrase, sPReverse);

This is how functions like strcpy() work, so there's precedent for doing this, and it's the least painful of all the options.

Just remember that string handling in C is very primitive (we're talking stone knives and bearskins here), and a lot of concepts that make sense in other languages (like using = to assign string contents) don't really apply in C.

Answer 3

Type char* sPhrase[] is really incompatible with char *, [] has semantic of pointer, so you code much to char **. So function signature must be:

char* reverse(char* sPhrase)

Answer 4

char sPhrase[STRMAX];

is a auto variable on stack and therefore it's adress can't be reassigned. should be decalred as:

char *sPhrase;

This defines an array of STRMAX pointers to char (as auto variable which will be destoryed after returning from function).

char* sOutput[STRMAX];

what you need here is:

char* sOutput = malloc( STRMAX );

Since you wan't you reverse duplicated string alive after you return.

Answer 5

I'm kind of new to pointers, and c in general, but it looks like reverse() is expecting a pointer and you're passing it an actual array of chars. try changing the line:

char sPhrase[STRMAX];

to:

char *sPhrase[STRMAX];

Answer 6

This works if I set the return type to void:

/*
 * code.c
 *
 * TASK
 *      Reverse a string by reversing pointers. Function should use return
 *      type char* and use a char* parameter as input.
 */
#include <stdio.h>
#include <string.h>
#define STRMAX 51

void reverse(char* sPhrase[]);

int main() {
    char sPhrase[STRMAX];
    char* sPPhrase[STRMAX];
    char* sPReverse[STRMAX];
    int iCntr;

    printf("Enter string (max. 50 chars): ");
    gets(sPhrase);

    for (iCntr = 0; iCntr < strlen(sPhrase); iCntr++) {
        sPPhrase[iCntr] = &sPhrase[iCntr];
    }

    reverse(sPPhrase);          // Disabled return type char*

    return 0;
}

void reverse(char* sPPhrase[]) {
    char* sPOutput[STRMAX];
    int iCnt = 0, iCntRev;

    for (iCntRev = strlen(*sPPhrase)-2; iCntRev >= 0; iCntRev--) {
        sPOutput[iCnt] = sPPhrase[iCntRev];
        iCnt++;
    }

    *sPOutput[iCnt] = '\0';      // Don't forget to close the string

    // return sPOutput;     // Disabled return type char*
}

As soon as I get the return types back into play, things go wrong. sPReverse = reverse(sPPhrase); still returns the incompatible types in assignment error even though the return type is the same as the type of the variable I'm storing the returned pointer array in.