Reversing a string in C

Question

I have developed a reverse-string program. I am wondering if there is a better way to do this, and if my code has any potential problems. I am looking to practice some advanced features of C.

char* reverse_string(char *str) {     char temp;     size_t len = strlen(str) - 1;     size_t i;     size_t k = len;      for(i = 0; i < len; i++)     {         temp = str[k];         str[k] = str[i];         str[i] = temp;         k--;          /* As 2 characters are changing place for each cycle of the loop            only traverse half the array of characters */         if(k == (len / 2))         {             break;         }     } } 

Answer 1

If you want to practice advanced features of C, how about pointers? We can toss in macros and xor-swap for fun too!

#include <string.h> // for strlen()  // reverse the given null-terminated string in place void inplace_reverse(char * str) {   if (str)   {     char * end = str + strlen(str) - 1;      // swap the values in the two given variables     // XXX: fails when a and b refer to same memory location #   define XOR_SWAP(a,b) do\     {\       a ^= b;\       b ^= a;\       a ^= b;\     } while (0)      // walk inwards from both ends of the string,      // swapping until we get to the middle     while (str < end)     {       XOR_SWAP(*str, *end);       str++;       end--;     } #   undef XOR_SWAP   } } 

A pointer (e.g. char *, read from right-to-left as a pointer to a char) is a data type in C that is used to refer to location in memory of another value. In this case, the location where a char is stored. We can dereference pointers by prefixing them with an *, which gives us the value stored at that location. So the value stored at str is *str.

We can do simple arithmetic with pointers. When we increment (or decrement) a pointer, we simply move it to refer to the next (or previous) memory location for that type of value. Incrementing pointers of different types may move the pointer by a different number of bytes because different values have different byte sizes in C.

Here, we use one pointer to refer to the first unprocessed char of the string (str) and another to refer to the last (end). We swap their values (*str and *end), and move the pointers inwards to the middle of the string. Once str >= end, either they both point to the same char, which means our original string had an odd length (and the middle char doesn't need to be reversed), or we've processed everything.

To do the swapping, I've defined a macro. Macros are text substitution done by the C preprocessor. They are very different from functions, and it's important to know the difference. When you call a function, the function operates on a copy of the values you give it. When you call a macro, it simply does a textual substitution - so the arguments you give it are used directly.

Since I only used the XOR_SWAP macro once, it was probably overkill to define it, but it made more clear what I was doing. After the C preprocessor expands the macro, the while loop looks like this:

    while (str < end)     {       do { *str ^= *end; *end ^= *str; *str ^= *end; } while (0);       str++;       end--;     } 

Note that the macro arguments show up once for each time they're used in the macro definition. This can be very useful - but can also break your code if used incorrectly. For example, if I had compressed the increment/decrement instructions and the macro call into a single line, like

      XOR_SWAP(*str++, *end--); 

Then this would expand to

      do { *str++ ^= *end--; *end-- ^= *str++; *str++ ^= *end--; } while (0); 

Which has triple the increment/decrement operations, and doesn't actually do the swap it's supposed to do.

While we're on the subject, you should know what xor (^) means. It's a basic arithmetic operation - like addition, subtraction, multiplication, division, except it's not usually taught in elementary school. It combines two integers bit by bit - like addition, but we don't care about the carry-overs. 1^1 = 0, 1^0 = 1, 0^1 = 1, 0^0 = 0.

A well known trick is to use xor to swap two values. This works because of three basic properties of xor: x ^ 0 = x, x ^ x = 0 and x ^ y = y ^ x for all values x and y. So say we have two variables a and b that are initially storing two values va and vb.

   // initially:   // a == va   // b == vb   a ^= b;   // now: a == va ^ vb   b ^= a;   // now: b == vb ^ (va ^ vb)   //        == va ^ (vb ^ vb)   //        == va ^ 0   //        == va   a ^= b;   // now: a == (va ^ vb) ^ va   //        == (va ^ va) ^ vb   //        == 0 ^ vb   //        == vb

So the values are swapped. This does have one bug - when a and b are the same variable:

   // initially:   // a == va   a ^= a;   // now: a == va ^ va   //        == 0   a ^= a;   // now: a == 0 ^ 0   //        == 0   a ^= a;   // now: a == 0 ^ 0   //        == 0 

Since we str < end, this never happens in the above code, so we're okay.

While we're concerned about correctness we should check our edge cases. The if (str) line should make sure we weren't given a NULL pointer for string. What about the empty string ""? Well strlen("") == 0, so we'll initialize end as str - 1, which means that the while (str < end) condition is never true, so we don't do anything. Which is correct.

There's a bunch of C to explore. Have fun with it!

Update: mmw brings up a good point, which is you do have to be slightly careful how you invoke this, as it does operate in-place.

 char stack_string[] = "This string is copied onto the stack.";  inplace_reverse(stack_string); 

This works fine, since stack_string is an array, whose contents are initialized to the given string constant. However

 char * string_literal = "This string is part of the executable.";  inplace_reverse(string_literal); 

Will cause your code to flame and die at runtime. That's because string_literal merely points to the string that is stored as part of your executable - which is normally memory that you are not allowed to edit by the OS. In a happier world, your compiler would know this, and cough an error when you tried to compile, telling you that string_literal needs to be of type char const * since you can't modify the contents. However, this is not the world my compiler lives in.

There are some hacks you could try to make sure that some memory is on the stack or in the heap (and is therefore editable), but they're not necessarily portable, and it could be pretty ugly. However, I'm more than happy to throw responsibility for this to the function invoker. I've told them that this function does in place memory manipulation, it's their responsibility to give me an argument that allows that.

Answer 2

Just a rearrangement, and safety check. I also removed your non-used return type. I think this is a safe and clean as it gets:

#include <stdio.h> #include <string.h>  void reverse_string(char *str) {     /* skip null */     if (str == 0)     {         return;     }      /* skip empty string */     if (*str == 0)     {         return;     }      /* get range */     char *start = str;     char *end = start + strlen(str) - 1; /* -1 for \0 */     char temp;      /* reverse */     while (end > start)     {         /* swap */         temp = *start;         *start = *end;         *end = temp;          /* move */         ++start;         --end;     } }   int main(void) {     char s1[] = "Reverse me!";     char s2[] = "abc";     char s3[] = "ab";     char s4[] = "a";     char s5[] = "";      reverse_string(0);      reverse_string(s1);     reverse_string(s2);     reverse_string(s3);     reverse_string(s4);     reverse_string(s5);      printf("%s\n", s1);     printf("%s\n", s2);     printf("%s\n", s3);     printf("%s\n", s4);     printf("%s\n", s5);      return 0; } 

Edited so that end will not point to a possibly bad memory location when strlen is 0.