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

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 2

The parameters does not match up with the variable names.

• sPhrase is declared as an array of chars.
• The function reverse has a parameter sPhrase which is a pointer to an array of chars

This explains why there is a 'type mismatch'. I have included a fixed version, try that.

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];*/
    static char sOutput[STRMAX];
    int iCnt = 0, iCntRev;

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

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

    return sOutput;
}

The compiler is pretty smart to check for 'incompatible' types. C is strict very relaxed when it comes to this kind of thing, so the onus is on you on making sure the declarations match up with the definitions in order for a successful compile.

I have also corrected the problem with using the sOutput variable within the reverse function, and changed it to a static as the variable will go out of scope and will end up with garbage, by prefixing the keyword static will ensure that the variable remains in scope.

Edit: For some reason, I added the strike tag to strike out the line but others are seeing it somehow...???

On closer inspection, the code does not work, I have fixed it, taking into account of other people's inputs and consideration, note I have used fgets instead of the evil gets function as that is a breeding ground for buffer overflows, also changed the function signature to use the const keyword for the parameter.

char * reverse(const char *sPhrase);

int main() {
    char sPhrase[STRMAX];
    char *sReverse;
    printf("Enter string (max. 50 chars): ");
    fgets(sPhrase, STRMAX - 1, stdin);
    sReverse = reverse(sPhrase);
    printf("Reversed string: %s\n", sReverse);
    return 0;
}

char *reverse(const char* sPhrase) {
    char* sOutput;
    int iCnt = 0, iCntRev;
    sOutput = (char *)malloc((STRMAX * sizeof(char)) + 1);
    if (sOutput){
        for (iCntRev = strlen(sPhrase) - 1; iCntRev >= 0; iCntRev--) {
            *sOutput++ = sPhrase[iCntRev];
            iCnt++;
        }
        *sOutput++ = '\0';
    }
    return (sOutput - iCnt);
}

Here's an excellent tutorial on pointers.

Hope this helps, Best regards, Tom.

Answer 3

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 4

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 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.

Answer 7

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.