Reversing a string using pointer-arithmetic

Question

I want to reverse a string using pointer arithmetic. I wrote a small program based on what I read in "A Book on C" and basic programs I wrote myself (which worked to my satisfaction). Here is what I wrote:

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

void rv(char* str)
{
    int l = strlen(str);
    char* copy = (char*) malloc((l + 1) * sizeof(char));
    assert(copy != NULL);
    int i;

    for (i = 0 ; i < l ; i++) { // copy str to copy in reversed order
        *(copy + i) = *(str + (l - 1 - i));
    }

    for (i = 0 ; i < l ; i++) { // 
        *(str + i) = *(copy + i);
    }

    free(copy);
    printf("Current string is %s\n", str);
}

int main(void)
{
    char* s1 = "abcde";
    rv(s1); 
    return 0;
}

The program crashes. From my tests, the problematic part is:

for (i = 0 ; i < l ; i++) { // 
    *(str + i) = *(copy + i);
}

I read similar questions, and it seems that this little piece of code should do no harm. actually, even if I replace that part with:

*(str + i) = 'c';

The program still crashes. It seems I can't even change one simple character of the string. what is the problem?

Just a thought, but, perhaps this assignment char *s1 = "abcde"; uses a fixed string, set as immutable? — Ross Presser
– Ross Presser, Commented Apr 9, 2014 at 19:16
Note that this can be done w/o allocating copy: swap str[0] & str[l-1], str[1] & str[l-2], ..., until (left as an exercise). Wouldn't solve the problem @RossPresser identified, though. — Scott Hunter
– Scott Hunter, Commented Apr 9, 2014 at 19:19
Is there a reason you're using pointer arithmetic rather than normal array indexing? I believe the compiler will almost certainly generate the same code either way. — dfeuer
– dfeuer, Commented Apr 9, 2014 at 19:50

jwaliszko · Accepted Answer · 2014-04-09 19:35:18Z

3

Here is the culprit:

for (i = 0 ; i < l ; i++) {
    *(str + i) = *(copy + i);
}

This crashes, because you are passing a non-writable string into the rv function.

To fix, copy the content of the string that you are reversing into a writable memory:

int main(void)
{
    char s1[] = "abcde";
    rv(s1); // No crash here!
    return 0;
}

With the crash out of the way, you could work on improving the efficiency of your algorithm: rather than allocating and freeing a temporary string, you could reverse in place by starting two pointers at both ends, swapping their content, and then moving them toward the middle. Stop once your pointers meet in the middle:

char *s = str;
char *e = s + strlen(str)-1;
for ( ; s < e ; s++, e--) {
    char tmp = *s;
    *s = *e;
    *e = tmp;
}

edited Apr 9, 2014 at 19:35

jwaliszko

17.1k22 gold badges95 silver badges161 bronze badges

answered Apr 9, 2014 at 19:19

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

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

Comments

Krzysztof Adamski · Accepted Answer · 2014-04-09 19:50:46Z

C compilers should place string literals (each string you write in your program literally, enclosing it inside of apostrophes) in read only region of memory. You are not allowed to modify it. In C, it's quite important to know what is the memory layout of your program. By using following line of code:

char* s1 = "abcde"

You basically tell your compiler to do two things:

Create a string literal "abcde", add it to your program in a section that should be placed in a read only memory when your program is executed.
Create a pointer to char, called s1, where you store a memory address pointing to the string literal created in 1.

Since you want to modify your string in place, you have to first copy it to the normal (read-write) memory. This can be done in couple ways:

Manually, by allocating read-write memory on heap, using malloc, then using strcpy to copy the string:
```
char *s1 = malloc(strlen("abcde")+1); // +1 for `\0` at the end of string
strcpy(s1, "abcde");
```
Automatically, by using C array initializer:
```
char s1[] = "abcde";
```

The second way works because of compiler "magic" (it will allocate a proper amount of memory and copy the string for you). It's important to understand that you can only do this when initializing an array, but you can't do this after initialization, so this, won't work:

char s1[];
s1 = "abcde";

Also note that what there is no guarantee that string literals are in read-only memory. Some compilers/systems may place them in general, read-write memory. Most compilers have some option that will do that. This doesn't matter, however, if you want your code to be compatible with standard C language and because of that, portable. A good rule of thumb is that you should always try to avoid implementation specifics when writing your code, if possible. But knowing when this is the case may probably the hardest part of learning C language.

Morten Jensen · Accepted Answer · 2014-04-09 19:24:35Z

0

You are trying to modify a const char array :)

Try allocating the memory as stack allocated, static or in global space instead.

int main(void)
{
    static char s1[] = "abcde";
    rv(s1); 
    return 0;
}

which outputs this on my machine:

morten@laptop:/tmp$ clang -Wall t.c 
morten@laptop:/tmp$ ./a.out 
Current string is edcba

answered Apr 9, 2014 at 19:24

Morten Jensen

5,9573 gold badges46 silver badges57 bronze badges

Comments

qulinxao · Accepted Answer · 2014-04-09 19:36:07Z

0

char* rv(char* s){
        char* l=s, *r=s+strlen(s)-1,t;
        while(l<r){
                t=*l;
                *l++=*r;
                *r--=t;
        }
        return s;
}

and ofcose s is pointer to writable row of char. gl.

answered Apr 9, 2014 at 19:36

qulinxao

3322 silver badges11 bronze badges

Collectives™ on Stack Overflow

Reversing a string using pointer-arithmetic

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related