How to reverse a string with pointers only?

Question

I'm trying to reverse a string, but it just stays the same. I don't use any modules except <string.h> and <stdio.h>.

void rev(s){
    char i, temp;
    char *sf = s;
    char ri = strlen((s) - 1);
    char *sl = &s[ri];
    for (i = 0; i < ri; i++){
        if (*sf != *sl){
            temp = *sf++;
            s[i] = *sl--; //
            s[ri--] = temp; //those two seems to be getting new characters, but it won't
        }
        else {
            ri--;
            sf++;
            sl--;
        }
    }
    printf("%s", s);
}

What does `char ri = strlen((s1) - 1);` do? There is no variable named `s1`. — mch, Sep 28 '20 at 12:02
If you swap a pair of things twice it looks unchanged. Look again at your code, you do that for each pair of characters. Only work on the pairs in which the first character is before middle. — Yunnosch, Sep 28 '20 at 12:12
@SouravGhosh I know you mean the right thing, but as you phrased it I read "123456" -> "321456". — Yunnosch, Sep 28 '20 at 12:14
@Yunnosch That's when you do it using pen and paper, when you refer the reversing logic in the code, you need to run till `len/2`..maybe I should have used swap instead of reverse there :) — Sourav Ghosh, Sep 28 '20 at 12:16

score 5 · Accepted Answer · answered Sep 28 '20 at 12:17

5

The function will not compile at least because the parameter does not have a type specifier.

void rev(s){

The type char has a little range of acceptable values. So you shall not use it for calculating the length of a string.

The call of strlen in this declaration

char ri = strlen((s) - 1);

invokes undefined behavior. It seems you mean

char ri = strlen(s) - 1;

that also can invoke undefined behavior for an empty string.

This loop

for (i = 0; i < ri; i++){

does not use pointers.

The function can be defined the following way as it is shown in the demonsytrative program below.

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

char * reverse( char *s )
{
    if ( *s )
    {
        for ( char *first = s, *last = s + strlen( s ); first < --last; ++first )
        {
            char c = *first;
            *first = *last;
            *last = c;
        }
    }
    
    return s;
}

int main( void ) 
{
    char s1[] = "1";
    char s2[] = "12";
    char s3[] = "123";
    
    puts( reverse( s1 ) );
    puts( reverse( s2 ) );
    puts( reverse( s3 ) );
}

The program output is

1
21
321

answered Sep 28 '20 at 12:17

Vlad from Moscow

301,070
26
186
335

@P__J__ I have not understood what you mean. Who did say that it is good? – Vlad from Moscow Sep 28 '20 at 12:21
@P__J__ One more I do not understand what you mean. In my answer all is written clear. So your comments do not mask a sense. – Vlad from Moscow Sep 28 '20 at 12:23
1

He is simply saying (I think) that you are creating and using a `char` variable to read the return of `strlen()`, ( in this statement: `char ri = strlen(s) - 1;` ) when [strlen()](https://www.tutorialspoint.com/c_standard_library/c_function_strlen.htm) returns `size_t`. But you already know this, so I am not sure what you are playing at. – ryyker Sep 28 '20 at 12:36
@P__J__ Ah yes, I see that now. Mis-read as `if ( s )` – chux - Reinstate Monica Sep 28 '20 at 19:24

0___________ · Answer 2 · 2020-09-28T22:14:34.290

1

Your code has plenty of issues (bugs ):

char ri = strlen((s) - 1); has to be size_t ri = strlen((s)) - 1;

Other code is very hard to analyze as you use not self explanatory variable names.

Here you have a bit more simple code and much easier to analyse.

char *reverseString(char *str)
{
    char *wrk = str, *end;
    if(str && *str)
    {
        end = str + strlen(str) - 1;
        while(end > wrk)
        {
            char temp = *wrk;
            *wrk++ = *end;
            *end-- = temp;
        }
    }
    return str;
}


int main(void)
{
    char str[] = "1234567890";
    printf("reversed: %s\n", reverseString(str));
}

edited Sep 28 '20 at 22:14

answered Sep 28 '20 at 12:13

0___________

60,014
4
34
74

I think you should explain why `if(str && *str)` cannot be replaced by `if(str)` – klutt Sep 28 '20 at 12:20
it does not make any sense to cal strlen if the string is empty. – 0___________ Sep 28 '20 at 12:21
Well, that's true, but that's not the reason to why it's necessary. – klutt Sep 28 '20 at 12:23
if(str) is not also necessary. Caller should take care . I do not understand your problem – 0___________ Sep 28 '20 at 12:26
If `str` is an empty string, then `end = str + strlen(str) - 1` would evaluate to `end = str -1` which would invoke UB – klutt Sep 28 '20 at 12:27
Besides, `strlen(NULL)` is also UB – klutt Sep 28 '20 at 12:30
@klutt there is no UB here as pointer can point one char before an array of after. Those pointers can be used in the pointer arithmetics or comparisons but they cannot be dereferenced. Later end > wrk will be false and no dereferencing. – 0___________ Sep 28 '20 at 12:31
@klutt as I wrote caller shoud make sure that pointer is valid as almost for most standard libraries functions. – 0___________ Sep 28 '20 at 12:32
1

One element before? Are you sure? I knew that one element AFTER was ok, but not one element before. – klutt Sep 28 '20 at 12:33
1

As far as I can see, you're wrong. Your function would invoke UB for empty string without the extra `&& *str` and the reason I'm bringing it up is that someone reading this would easily believe that you could safely remove that. – klutt Sep 28 '20 at 12:40
@klutt does my code invoke any UB? I do not understand your nitpicks – 0___________ Sep 28 '20 at 19:21
Nope, it does not. But it would if you changed `if(str && *str)` to `if(str)`. Then it would be UB for empty string. And my point is that for someone who is not aware of that, it would be easy to think that they could safely do that change. I would personally have sacrificed that optimization for cleaner code if I did not know about that pitfall. – klutt Sep 28 '20 at 21:03
So what I'm saying is that it is worth mentioning that `&& *str` actually is necessary. It's not only an optimization. – klutt Sep 28 '20 at 21:04

CiaPan · Answer 3 · 2020-09-28T19:17:53.120

1

A simple solution:

char *sl = sf;
while (*sl != 0)
    ++ sl;
-- sl; 
while (sf < sl)
{
    char c = *sf;
    *sf = *sl;
    *sl = c;

    ++sf, --sl;
}

Find the end of the string by skipping all characters until you find the NUL (zero) character.
Then step back one character (decrement sl) so that you have pointers to the first and the last character of the string.

Then walk both pointers towards one another and swap characters until the pointers meet or cross.

edited Sep 28 '20 at 19:17

answered Sep 28 '20 at 12:13

CiaPan

9,381
2
21
35

1

Hmmm, putting `--sl` in the init part of for loop just seems confusing even if it is correct – klutt Sep 28 '20 at 12:17
@klutt Changed. – CiaPan Sep 28 '20 at 19:18
Better, but this actually have a bug. For an empty string, this would cause UB, because `--sl` would then be `sf - 1`. You could solve this by adding `if(*sf == '\0') return sf` somewhere before `--sf`. Either before or after the while loop. – klutt Sep 28 '20 at 20:59

How to reverse a string with pointers only?

3 Answers3