String reversal from K & R 2nd edition

[ashok.anbalan]

I tried using the string reversal routine that occurs in K & R 2nd
edition (Pg. 62) & the program core dumps.

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

void reverse ( char s[] )
{
int c, i, j;

printf("\nInput string is %s",s);

for (i=0, j= strlen(s)-1;i<j;i++,j--)
{
c = s;
s = s[j];
s[j] = c;
}

printf("\nReversed string is %s",s);
}

int main()
{
reverse("abcd");
}

Apart from the two prints, this is an exact copy of the routine. I am
using gcc on Cygwin.

Can someone tell me whats going wrong?

Thanks,
Ashok

 

[David Resnick]

I tried using the string reversal routine that occurs in K & R 2nd
edition (Pg. 62) & the program core dumps.

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

void reverse ( char s[] )
{
int c, i, j;

printf("\nInput string is %s",s);

for (i=0, j= strlen(s)-1;i<j;i++,j--)
{
c = s;
s = s[j];
s[j] = c;
}

printf("\nReversed string is %s",s);
}

int main()
{
reverse("abcd");
}

Apart from the two prints, this is an exact copy of the routine. I am
using gcc on Cygwin.

Can someone tell me whats going wrong?

Thanks,
Ashok

You are attempting to modify a string literal, can't do that.

try this:
int main(void)
{
char foo[] = "abcd";
reverse(foo);
return 0;
}

You should put a '\n' at the end of your printfs
rather than the beginning, the last one may
or may not be displayed.

You should check for a 0 length string too, not
if strlen(s) is 0;

-David

 

[ashok.anbalan]

Thanks David. Indeed the error was that a string literal was attempted
to be modified.

Here is what I finally got.

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

void reverse ( char *str )
{
int i; int len = strlen(str);
if (!len) return;
printf("Input string is %s \n", str);
for ( i=0; i < len/2; i++)
{
*(str + len-1-i) ^= *(str+i) ^= *(str + len -1-i) ^=
*(str+i);
}

printf("Output string is %s \n", str);

}

int main ()
{
char mystr[] = "abcde";
reverse(mystr);
}

Is the strlen implemented here same as what you are referring to?

Ashok

 

[Chris Torek]

Here is what I finally got.
[snippage]

*(str + len-1-i) ^= *(str+i) ^= *(str + len -1-i) ^= *(str+i);

"Don't do that":

Archive-name: C-faq/faq
Comp-lang-c-archive-name: C-FAQ-list
URL: http://www.eskimo.com/~scs/C-faq/top.html

[Last modified July 3, 2004 by scs.]

3.3b: Here's a slick expression:

a ^= b ^= a ^= b

It swaps a and b without using a temporary.

A: Not portably, it doesn't. It attempts to modify the variable a
twice between sequence points, so its behavior is undefined.

For example, it has been reported that when given the code

int a = 123, b = 7654;
a ^= b ^= a ^= b;

the SCO Optimizing C compiler (icc) sets b to 123 and a to 0.

See also questions 3.1, 3.8, 10.3, and 20.15c.

 

[David Stevenson]

void reverse ( char *str )
{
int i; int len = strlen(str);
if (!len) return;
printf("Input string is %s \n", str);
for ( i=0; i < len/2; i++)
{
*(str + len-1-i) ^= *(str+i) ^= *(str + len -1-i) ^=
*(str+i);
}

printf("Output string is %s \n", str);

}

A simpler pointer solution. It appeared that you wanted something
optimized, and pointers can be better than indexes. But maybe
compilers are smarter now.

void reverse ( char *s )
{
char c ;
char *s1, *s2 ;

printf("Input string is %s\n",s);

for ( s1=s, s2=s+strlen(s)-1 ; s1 < s2 ; s1++, s2-- )
{
c = *s1 ;
*s1 = *s2 ;
*s2 = c ;
}


printf("Reversed string is %s\n",s);
}

David Stevenson

 

[pete]

David Stevenson wrote:

void reverse ( char *s )
{

char *s1, *s2 ;

s2=s+strlen(s)-1 ;

That's no good for when s points to a zero length string.

 

[Robert Bachmann]

void reverse ( char *s )
{
[...]
for ( s1=s, s2=s+strlen(s)-1 ; s1 < s2 ; s1++, s2-- )

As pete already pointed out:
s2=s+strlem(s)-1; is bad when strlen(s) == 0.

So here is a way to fix this (based on David Stevenson's code):

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

char* reverse ( char *s )
{
char c ;
char *s1, *s2 ;

assert(s != NULL);

if (*s == '\0')
return s;

for ( s1=s, s2=s+strlen(s)-1 ; s1 < s2 ; s1++, s2-- )
{
c = *s1 ;
*s1 = *s2 ;
*s2 = c ;
}

return s;
}

int main(void)
{
char s[]="abc";

printf("Original string: %s\n",s);
printf("Reversed string: %s\n",reverse(s));

return 0;
}

Note:
I've added a return type (char*) to reverse(), and added the assertion
that s shouldn't be NULL, as both of this additions didn't seem like a
bad idea for me.