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

I never use the char string[] syntax for arguments, since it makes it not obvious that string is actually a pointer. (Edit: I mean to the person reading or writing the code; the computer doesn't care) — Stack Exchange Broke The Law
– Stack Exchange Broke The Law, Commented Nov 21, 2015 at 0:55
in order to "make it obvious" should it be: char* string[] ? — kandaspohn
– kandaspohn, Commented Nov 21, 2015 at 0:56
It would be char* string. Next question: Do you know why void f(int x) {x = 7;} int main() {int y = 5; f(y); printf("%d\n", y); return 0;} prints 5 and not 7? — Stack Exchange Broke The Law
– Stack Exchange Broke The Law, Commented Nov 21, 2015 at 0:59
more generally: what happens if you return a pointer to local variable? — jev
– jev, Commented Nov 21, 2015 at 1:04
i'm really new to this so go easy on me but....is it because y points to a value and not the address of ? — kandaspohn
– kandaspohn, Commented Nov 21, 2015 at 1:04

John Mark Gabriel Caguicla · Accepted Answer · 2015-11-21 01:45:42Z

2

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.

edited Nov 21, 2015 at 1:45

answered Nov 21, 2015 at 1:18

John Mark Gabriel Caguicla

4492 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

John Mark Gabriel Caguicla Over a year ago

In the original code it the line array (char line[strlen(string)])was declared on the stack not on the heap, therefore it disappears after the function call leaving string pointing nowhere.

John Mark Gabriel Caguicla Over a year ago

@BobbySacamano Ah yes, that was my bad. You are correct about string = line since string was indeed passed on the stack. Got confused by char string[], was never used to passing arrays. Cheers.

kandaspohn Over a year ago

One last question John...what does adding the +1 to char line[strlen(string) + 1] do exactly? Is that what gets the char at 0?

John Mark Gabriel Caguicla Over a year ago

For example let's say we are passing "message" to reverse, that means strlen() will return 7, but C strings should be null terminated so there has to be a 0 at the very last character so you need one last slot for it (therefore char line[8]).

John Mark Gabriel Caguicla Over a year ago

@BobbySacamano No it does not, I just used it for the sake of explanation.

|

bruceg · Accepted Answer · 2015-11-21 01:34:53Z

1

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));
}

answered Nov 21, 2015 at 1:34

bruceg

2,5431 gold badge23 silver badges30 bronze badges

5 Comments

kandaspohn Over a year ago

can you explain what memcpy() does?

Bobby Sacamano Over a year ago

it copies memory from the second operand to the first operand, for the number of bytes specified by the third operand. sizeof(char) == 1 always, in case you see string examples missing the sizeof. With other data types you need sizeof.

bruceg Over a year ago

memcpy is a standard c function that copies the data from the 2nd parameter (the source) into the first parameter (the destination). The amount of data in bytes is specified in the 3rd parameter. In the code above it will copy the characters from the variable "line" into the memory location of the variable "string".

Bobby Sacamano Over a year ago

If someone calls reverse with a string literal, that will break things, btw.

bruceg Over a year ago

Right. That will cause a crash.

ChuckCottrill · Accepted Answer · 2015-11-21 02:21:11Z

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);
    }
}

Bobby Sacamano · Accepted Answer · 2015-11-21 05:22:57Z

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.

Collectives™ on Stack Overflow

loop to reverse string in C

4 Answers 4

6 Comments

5 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

6 Comments

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related