loop to reverse string in C

Question

So I've looked around on SO and can't find code that answers my question. I have written a function that is supposed to reverse a string as input in cmd-line. Here is the function:

void reverse (char string[]) {
    int x;
    int i = 0;
    char line[strlen(string)];

    for (x = strlen(string) - 1; x > 0; x--) {
        char tmp = string[x];
        line[i] = tmp;
        i++;
    }
    string = line;
}

When I call my reverse() function, the string stays the same. i.e., 'abc' remains 'abc'

If more info is needed or question is inappropriate, let me know.

Thanks!!

Answer 1

You're declaring your line array one char shorter remember the null at the end.

Another point, it should be for (x = strlen(string) - 1; x >= 0; x--) since you need to copy the character at 0.

void reverse (char string[]) {
    int x;
    int i = 0;
    char line[strlen(string) + 1];

    for (x = strlen(string) - 1; x >= 0; x--) {
        char tmp = string[x];
        line[i] = tmp;
        i++;
    }

    for(x = 0; x < strlen(string); x++)
    {
        string[x] = line[x];
    }
}

Note that this function will cause an apocalypse when passed an empty string or a string literal (as Bobby Sacamano said).

Suggestion you can probably do: void reverse(char source[], char[] dest) and do checks if the source string is empty.

Answer 2

The last line in your code does nothing string = line;

Parameters are passed by value, so if you change their value, that is only local to the function. Pointers are the value of the address of memory they are pointing to. If you want to modify the pointer that the function was passed, you need to take a pointer to that pointer.

Here is a short example of how you could do that.

void reverse (char **string) {
    char line = malloc(strlen(*string) + 1);
    //automatic arrays are deallocated once the function ends
    //so line needs to be dynamically or statically allocated

   // do something to line

    *string = line;
}

The obvious issue with this is that you can initialize the string with static memory, then this method will replace the static memory with dynamic memory, and then you'll have to free the dynamic memory. There's nothing functionally wrong with that, it's just a bit dangerous, since accidentally freeing the string literal is illegal.

char *test = "hello";
reverse(test);
free(test); //this is pretty scary

Also, if test was allocated as dynamic memory, the pointer to it would be lost and then it would become a memory leak.

Answer 3

I think that your answer is almost correct. You don't actually need an extra slot for the null character in line. You just need two minor changes:

• Change the assignment statement at the bottom of the procedure to a memcpy.
• Change the loop condition to <-

So, your correct code is this:

void reverse (char string[]) {
  int x;
  int i = 0;
  char line[strlen(string)];

  for (x = strlen(string) - 1; x >= 0; x--) {
    char tmp = string[x];
    line[i] = tmp;
    i++;
  }
  memcpy(string, line, sizeof(char) * strlen(line));
}

Answer 4

Since you want to reverse a string, you first must decide whether you want to reverse a copy of the string, or reverse the string in-situ (in place). Since you asked about this in 'C' context, assume you mean to change the existing string (reverse the existing string) and make a copy of the string in the calling function if you want to preserve the original.

You will need the string library

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

Array indexing works, and this version takes that approach,

/* this first version uses array indexing */
char*
streverse_a(char string[])
{
    int len; /*how big is your string*/
    int ndx; /*because 'i' is hard to search for*/
    char tmp; /*hold character to swap*/
    if(!string) return(string); /*avoid NULL*/
    if( (len=strlen(string)) < 2 ) return(string); /*one and done*/
    for( ndx=0; ndx<len/2; ndx++ ) {
        tmp=string[ndx];
        string[ndx]=string[len-1-ndx];
        string[len-1-ndx]=tmp;
    }
    return(string);
}

But you can do the same with pointers,

/* this is how K&R would write the function with pointers */
char*
streverse(char* sp)
{
    int len, ndx; /*how big is your string */
    char tmp, *bp, *ep; /*pointers to begin/end, swap temporary*/
    if(!sp) return(sp); /*avoid NULL*/
    if( (len=strlen(bp=sp)) < 2 ) return(sp); /*one and done*/
    for( ep=bp+len-1; bp<ep; bp++, ep-- ) {
        tmp=*bp; *bp=*ep; *ep=tmp; /*swap*/
    }
    return(sp);
}

(No, really, the compiler does not charge less for returning void.)

And because you always test your code,

char s[][100] = {
    "", "A", "AB", "ABC", "ABCD", "ABCDE",
    "hello, world", "goodbye, cruel world", "pwnz0r3d", "enough"
};

int
main()
{
    /* suppose your string is declared as 'a' */
    char a[100];
    strcpy(a,"reverse string");

    /*make a copy of 'a', declared the same as a[]*/
    char b[100];
    strcpy(b,a);
    streverse_a(b);
    printf("a:%s, r:%s\n",a,b);

    /*duplicate 'a'*/
    char *rp = strdup(a);
    streverse(rp);
    printf("a:%s, r:%s\n",a,rp);
    free(rp);

    int ndx;
    for( ndx=0; ndx<10; ++ndx ) {
        /*make a copy of 's', declared the same as s[]*/
        char b[100];
        strcpy(b,s[ndx]);
        streverse_a(b);
        printf("s:%s, r:%s\n",s[ndx],b);

        /*duplicate 's'*/
        char *rp = strdup(s[ndx]);
        streverse(rp);
        printf("s:%s, r:%s\n",s[ndx],rp);
        free(rp);
    }
}