C String Reversed

Question

I am writing a simple c program which reverses a string, taking the string from argv[1]. Here is the code:

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

char* flip_string(char *string){
    int i = strlen(string);
    int j = 0;

    // Doesn't really matter all I wanted was the same size string for temp.
    char* temp = string;
    puts("This is the original string");
    puts(string);
    puts("This is the \"temp\" string");
    puts(temp);

    for(i; i>=0; i--){
        temp[j] = string[i]
        if (j <= strlen(string)) {
            j++;
        }
    }

    return(temp);
}

int main(int argc, char *argv[]){
    puts(flip_string(argv[1]));
    printf("This is the end of the program\n");
}

That's basically it, the program compiles and everything but does not return the temp string in the end (just blank space). In the beginning it prints temp fine when its equal to string. Furthermore if I do a character by character printf of temp in the for loop the correct temp string in printed i.e. string -> reversed. Just when I try to print it to standard out (after the for loop/ or in the main) nothing happens only blank space is printed.

thanks

Answer 1

Your function appears to do an 'in-place reversal', i.e. it replaces the given string in memory. When doing so, make sure that you do not move the trailing zero to the front. Here's a simple implementation of the function

#include <assert.h>
#include <string.h>

void flip_string(char *s)
{
    assert(s);
    char *t = strchr(s, '\0') - 1;
    for (; s < t; ++s, --t) {
        char tmp = *s;
        *s = *t;
        *t = tmp;
    }
}

The function asserts that it gets a string (i.e. not a null pointer) and that the memory is writable. It then sets up a t pointer to point to the last character of the string - doing this via strchr instead of writing a manual loop has the benefit that strchr is usually a highly optimized function which doesn't traverse the string in single-byte steps but rather considers four or even more bytes at a time. It's also arguably more expressive.

The main loop then swaps the characters referenced by s and t (i.e. initially the first and the last character) and then moves the pointers forwards/backwards until they meet.

The function is a bit more concise because it doesn't need to retain the original pointer passed in, instead it can modify s directly. This is a result of deciding that a function should either modify its argument or return a new value - but not both. Doing both means you could call the function like

printf("%s", flip_string(s));

...and this would totally obscure that s is actually modified.

flip_string(s);
printf("%s", s);

is more explicit in that respect.

Answer 2

Here, you obviously want temp to be an other variable than string. But the initialization you made will result in the two pointers pointing to the same location.

What you should do instead is :

char *flip_string(const char *string)
{
    char *tmp = NULL;
    size_t len = strlen(string);
    int i = 0;
    /*
     * Don't forget that strlen() returns the length of the string
     * **WITHOUT** counting the ending '\0' character.
     * Thus, you have to add the extra space when you're allocating
     */
    if (!(tmp = malloc(len + 1))) {
        printf("Allocation failed ...\n");
        return NULL;
    }
    /*
     * The newly created string must have an ending character
     * Which is '\0'
     */

    tmp[len] = '\0';

    /*
     * Here, you want to reverse the "true content" of your string.
     * That is why you ignore the ending '\0' of the string by
     * setting the upper bound to strlen (with a strict '<') **AND**
     * it is also the reason a '- 1' just pops in the index choice.
     */
    for(i = 0; i < len; i++) {
        tmp[i] = string[len - i - 1];
    }
    return tmp;
}

As underlined by WhozCraig, there is an alternative solution which simply modifies the argument string without requiring memory allocation :

void flip_string(char *s)
{
    size_t len = strlen(s);
    char *p = s + len - 1;
    while (s < p) {
        *p ^= *s;
        *s ^= *p;
        *p ^= *s;
        p--;
        s++;
    }
}

Note the XOR trick to avoid using a temporary storage variable for the swapped character (^ is the XOR operator in C)

Answer 3

You were going in the correct direction trying to use a pointer. Just a little more thought and you probably would have had it. A safe implementation appears below:

#include <stdio.h>

char *flip_string(char *str)
{
    char *lhs = str, *rhs = str;
    if (!str || !*str || !*(str+1))
        return str;

    while (*++rhs); // rhs to eos
    while (lhs < --rhs)
    {
        char tmp = *lhs;
        *lhs++ = *rhs;
        *rhs = tmp;
    }
    return str;
}

int main()
{
    char test1[] = "Hello, World!";
    char test2[] = "";
    char test3[] = "1";
    printf("%s, %s, %s\n", flip_string(test1), flip_string(test2), flip_string(test3));
    return 0;
}

Output

!dlroW ,olleH, , 1

Hope it helps.