Highly efficient string reversal code

Hello, I'm a highly experienced expert C programmer

No, you are not.

and I've written
this code to reverse a string in place. I think you could all learn
something from it!

I doubt it. You should rethink it. It is not, as you subject line
claims "highly efficient".

Sep 22 '08 #2

Lew Pitcher

On September 22, 2008 14:42, in comp.lang.c, do**************@ymail.com
(do**************@ymail.com) wrote:

Hello, I'm a highly experienced expert C programmer

Apparently, you are
a) egotistical, and
b) wrong

and I've written
this code to reverse a string in place.

I am /so/ sorry, both for you and your employer.

I think you could all learn
something from it!

I'm sure that you are right. After all, /I/ learned that you are much less
than the C programmer that you claim to be.

int reverse(char* reverseme){
int retval=-1;
if(retval!=NULL)

Useless test, as retval was initialized (and never altered) to a value
that will not equate to NULL

{
int len=strlen(retval)

First real failure. retval is an integer, and the argument to
strlen() is a pointer-to-char. You can't get a valid result from
taking the "string length" of an integer.

Second failure: you did not #include the header that declares the
strlen() function. Thus, your compiler did not flag the above
assignment as an error (argument of incompatable type)

{
if(len>0)

So, what is the "string length" of an integer? And when would it be
greater than zero?

{
int half=len>>1;

Style hack: To be clearer to the intent of what sort of number
half represents, you should have coded this as
int half = len / 2;
Of course, since len is a nonsense value, half will also be a
nonsense value.

for(;retval<half;retval++){
reverseme[retval]^=reverseme[len-(retval+1)];

Undefined behaviour on first iteration, when retval == -1
reverseme[-1] is out of bounds.

reverseme[len-(retval+1)]=reverseme[retval];

Undefined behaviour on first iteration - same reason

reverseme[retval]^=reverseme[len-(retval+1)];

Undefined behaviour on first iteration - same reason

Plus, you've screwed up the "xor trick", and wiped out /both/
ends of the string with invalid values.

The head end now contains an array of 0x00 (up to
the ill-computed "midpoint"), while the tail end contains the
results of the head end xor the tail end.

}
}
}
return retval;

Of what use is this return value?

}

-Phil/CERisE

--
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
---------- Slackware - Because I know what I'm doing. ------

Sep 22 '08 #3

Eric Sosman

Hello, I'm a highly experienced expert C programmer [...]

s/C programmer/PLONKed troll/

--
Er*********@sun.com

Sep 22 '08 #4

Default User

Hello, I'm a highly experienced expert C programmer and I've written
this code to reverse a string in place. I think you could all learn
something from it!

You should learn some better trolling techniques. I suggest studying
under Bill Cunningham.

Brian

Sep 22 '08 #5

Flash Gordon

Lew Pitcher wrote, On 22/09/08 20:06:

On September 22, 2008 14:42, in comp.lang.c, do**************@ymail.com
(do**************@ymail.com) wrote:

>Hello, I'm a highly experienced expert C programmer

Apparently, you are
a) egotistical, and
b) wrong

Agreed. Although you missed some of the problems...

>and I've written
this code to reverse a string in place.

I am /so/ sorry, both for you and your employer.

You think s/he is employed?

>I think you could all learn
something from it!

I'm sure that you are right. After all, /I/ learned that you are much less
than the C programmer that you claim to be.

>int reverse(char* reverseme){
int retval=-1;
if(retval!=NULL)

Useless test, as retval was initialized (and never altered) to a value
that will not equate to NULL

<snip load more broken code>

You forgot to mention that as retval is an int it won't actually compile
on all implementations.

I think it was a troll. However, it was amusing for how bad the code
was. I wonder how many other errors it has that have been missed?
--
Flash Gordon
If spamming me sent it to sm**@spam.causeway.com
If emailing me use my reply-to address
See the comp.lang.c Wiki hosted by me at http://clc-wiki.net/

Sep 22 '08 #6

Lew Pitcher <lp******@teksavvy.comwrites:

On September 22, 2008 14:42, in comp.lang.c, do**************@ymail.com
(do**************@ymail.com) wrote:

[...]

> int retval=-1;
if(retval!=NULL)

Useless test, as retval was initialized (and never altered) to a value
that will not equate to NULL

[...]

It's not just useless. retval is an int; NULL expands to a null
pointer constant. If NULL happens to be #defined as 0, the test is
merely useless; if it's #defined as ((void*)0) or something similar,
the test is a constraint violation.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Sep 22 '08 #7

Flash Gordon <sm**@spam.causeway.comwrites:
[...]

I think it was a troll. However, it was amusing for how bad the code
was. I wonder how many other errors it has that have been missed?

I don't think anybody has mentioned the syntax error yet.
int len=strlen(retval){
should be
int len=strlen(retval);

I'd call it a comedy of errors, but that would require it to be funny.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Sep 22 '08 #8

dj3vande

In article <dc**************************@TEKSAVVY.COM-Free>,
Lew Pitcher <lp******@teksavvy.comwrote:

>On September 22, 2008 14:42, in comp.lang.c, do**************@ymail.com
(do**************@ymail.com) wrote:

>Hello, I'm a highly experienced expert C programmer

Apparently, you are
a) egotistical, and
b) wrong

>and I've written
this code to reverse a string in place.

I am /so/ sorry, both for you and your employer.

>I think you could all learn
something from it!

I'm sure that you are right. After all, /I/ learned that you are much less
than the C programmer that you claim to be.

Since the subject has come up, here's one I've been waiting for an
excuse to post:
--------
typedef struct l{char a;struct l*b;}l;
static void x(char*a,l*b) {if(b){(-1)[a]=b->a;x(--a,b->b);}}
static void y(char*a,l*b,l*c) {if(b){l*d=b->b;b->b=c;y(a,d,b);}else{x(a,c);}}
static void z(char*a,l*b) {if(*a){l c;c.a=*a;c.b=b;z(a+1,&c);}else{y(a,b,0);}}
void reverse(char *s) { z(s,0); }
--------
Unlike the one that started the thread, this has actually been tested
and verified to work.

It also uses advanced implementation techniques that allow programs
with certain properties (especially in languages with properties that
make those program properties easy to detect and exploit) to be
compiled to highly efficient code.

Somebody who spends enough time untangling it might even learn
something useful from it (for sufficiently, but not comically, loose
values of "useful").
dave
(bonus points if you can figure out where the idea to do it that way came from)

--
Dave Vandervies dj3vande at eskimo dot com

None of my copies of K&R have a "screen". Should I apply for a refund?
--Chris Dollin in comp.lang.c

Sep 22 '08 #9

Jens Stueckelberger

On Mon, 22 Sep 2008 11:42:37 -0700, dominantubergeek wrote:

Hello, I'm a highly experienced expert C programmer and I've written
this code to reverse a string in place. I think you could all learn
something from it!

Boy, that last sure is going to endear you to people here!

Sep 22 '08 #10

Richard

Jens Stueckelberger <JS*************@nowhere.orgwrites:

On Mon, 22 Sep 2008 11:42:37 -0700, dominantubergeek wrote:

>Hello, I'm a highly experienced expert C programmer and I've written
this code to reverse a string in place. I think you could all learn
something from it!

Boy, that last sure is going to endear you to people here!

Suggesting people show new programmers pointers at work in a debugger
endears you to some of the dustier c.l.c dweller ... :-;

Sep 22 '08 #11

Bartc

<dj******@csclub.uwaterloo.ca.invalidwrote in message
news:gb**********@rumours.uwaterloo.ca...

Since the subject has come up, here's one I've been waiting for an
excuse to post:
--------
typedef struct l{char a;struct l*b;}l;
static void x(char*a,l*b) {if(b){(-1)[a]=b->a;x(--a,b->b);}}
static void y(char*a,l*b,l*c)
{if(b){l*d=b->b;b->b=c;y(a,d,b);}else{x(a,c);}}
static void z(char*a,l*b) {if(*a){l
c;c.a=*a;c.b=b;z(a+1,&c);}else{y(a,b,0);}}
void reverse(char *s) { z(s,0); }
--------
Unlike the one that started the thread, this has actually been tested
and verified to work.

It also uses advanced implementation techniques that allow programs
with certain properties (especially in languages with properties that
make those program properties easy to detect and exploit) to be
compiled to highly efficient code.

Somebody who spends enough time untangling it might even learn
something useful from it (for sufficiently, but not comically, loose
values of "useful").

I tested your reverse function against the simplest reverse function of my
own I could knock up. Mine was 6 to 20 times faster that yours, to reverse a
77-character string one million times on my machine.

So if there is a point to your version, I haven't quite grasped it.

void myreverse(char *s) {
int i,j;
char c;

if (s==NULL) return;

j=strlen(s)-1;
i=0;

while (i<j) {
c=s[i];
s[i]=s[j];
s[j]=c;
++i;
--j;
}

}

--
Bartc

Sep 22 '08 #12

>
Hello, I'm a highly experienced expert C programmer and I've
written this code to reverse a string in place. I think you could
all learn something from it!

int reverse(char* reverseme){
int retval=-1;
if(retval!=NULL){
int len=strlen(retval){
if(len>0){
int half=len>>1;
for(;retval<half;retval++){
reverseme[retval]^=reverseme[len-(retval+1)];
reverseme[len-(retval+1)]=reverseme[retval];
reverseme[retval]^=reverseme[len-(retval+1)];
}
}
}
return retval;
}

This is so horrible and faulty that I have to attach the
following. Don't forget to #include <strings.h>. Note that it
returns strlen.

/* ======================= */
/* reverse string in place */
size_t revstring(char *stg)
{
char *last, temp;
size_t lgh;

lgh = strlen(stg);
if (lgh 1) {
last = stg + lgh; /* points to '\0' */
while (last-- stg) {
temp = *stg; *stg++ = *last; *last = temp;
}
}
return lgh;
} /* revstring */

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 22 '08 #13

dj3vande

In article <9D******************@text.news.virginmedia.com> ,
Bartc <bc@freeuk.comwrote:

>
<dj******@csclub.uwaterloo.ca.invalidwrote in message
news:gb**********@rumours.uwaterloo.ca...

>Since the subject has come up, here's one I've been waiting for an
excuse to post:

[...]

>I tested your reverse function against the simplest reverse function of my
own I could knock up. Mine was 6 to 20 times faster that yours, to reverse a
77-character string one million times on my machine.

So if there is a point to your version, I haven't quite grasped it.

I'd rather give my version to somebody who's posting an obvious
homework problem than yours.

Once you untangle it, it also demonstrates a few code-structure ideas
that are quite useful to have wrapped your brain around (not that
reversing a string in C is really a _good_ way to illustrate those),
and I like to think it provides a nifty example of how to build working
approximations to some high-level abstractions that C doesn't provide
natively.
dave
(specifically NOT claiming that "nifty" implies "useful in production code".)

--
Dave Vandervies dj3vande at eskimo dot com

None of my copies of K&R have a "screen". Should I apply for a refund?
--Chris Dollin in comp.lang.c

Sep 22 '08 #14

Richard

CBFalconer <cb********@yahoo.comwrites:

do**************@ymail.com wrote:
>>
Hello, I'm a highly experienced expert C programmer and I've
written this code to reverse a string in place. I think you could
all learn something from it!

int reverse(char* reverseme){
int retval=-1;
if(retval!=NULL){
int len=strlen(retval){
if(len>0){
int half=len>>1;
for(;retval<half;retval++){
reverseme[retval]^=reverseme[len-(retval+1)];
reverseme[len-(retval+1)]=reverseme[retval];
reverseme[retval]^=reverseme[len-(retval+1)];
}
}
}
return retval;
}

This is so horrible and faulty that I have to attach the
following. Don't forget to #include <strings.h>. Note that it
returns strlen.

/* ======================= */
/* reverse string in place */
size_t revstring(char *stg)
{
char *last, temp;
size_t lgh;

lgh = strlen(stg);
if (lgh 1) {
last = stg + lgh; /* points to '\0' */
while (last-- stg) {
temp = *stg; *stg++ = *last; *last = temp;
}
}
return lgh;
} /* revstring */

You know I really had to look hard at this to understand it. Horrible
variables names almost purposely made "no standard" as far as C around
the world goes for such a function. Unnecessary length
check. Unnecessary size_t. Multiple assignments on one line
making perusal with a debugger almost impossible.

Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

Sep 22 '08 #15

CBFalconer <cb********@yahoo.comwrites:
<snip code>

This is so horrible and faulty that I have to attach the
following. Don't forget to #include <strings.h>.

I'd include <string.hinstead. I haven't seen strings.h for decades.

--
Ben.

Sep 22 '08 #16

Richard<rg****@gmail.comwrites:
<snip>

Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

Testing it would not reveal the issue.

--
Ben.

Sep 22 '08 #17

Richard

Ben Bacarisse <be********@bsb.me.ukwrites:

Richard<rg****@gmail.comwrites:
<snip>
>Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

>but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

Testing it would not reveal the issue.

Why wouldn't it? (and I acknowledge the correction btw).

Had I run this through a debugger with s as an empty string (I must
admit I had assumed s had to be non null and a non empty string but fair
enough point you raise) I would spot it in about 1 second when e is
calculated.

But correction (still no null check):
#include <stdio.h>
#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
t = *s;
*s++ = *e;
*e = t;
}
}

int main() {
char s1[]="hello";
my_strrev(s1);
printf("%s\n",s1);
char s2[]="";
my_strrev(s2);
printf("%s\n",s2);
char s3[]="c.l.c";
my_strrev(s3);
printf("%s\n",s3);
}

Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

Sep 22 '08 #18

pete

Richard wrote:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

It's undefined when attempting to reverse a zero length string.

--
pete

Sep 22 '08 #19

Richard

pete <pf*****@mindspring.comwrites:

Richard wrote:

>#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

It's undefined when attempting to reverse a zero length string.

Yes, I acknowledged that in my reply to Ben. Careless of me.

Sep 22 '08 #20

pete

Richard wrote:

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){

Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value

--
pete

Sep 22 '08 #21

Richard

pete <pf*****@mindspring.comwrites:

Richard wrote:

>char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){

>Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value

So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

Sep 22 '08 #22

Richard

Richard<rg****@gmail.comwrites:

pete <pf*****@mindspring.comwrites:

>Richard wrote:

>>char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){

>>Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value

So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

or to make my question more clear - does this mean the entire program is
"undefined"?

I must admit I would need to revisit some code (should I ever think it
will be compiled on a system where this really will not work) if that
"--" causes my frying pan to catch fire.

Sep 22 '08 #23

pete

Richard wrote:

Richard<rg****@gmail.comwrites:

>pete <pf*****@mindspring.comwrites:

>>Richard wrote:

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?
That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value
So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

Yes.

or to make my question more clear - does this mean the entire program is
"undefined"?

Yes.

There is a special rule that allows
the calculation of the "one past" pointer value.

--
pete

Sep 22 '08 #24

Richard

pete <pf*****@mindspring.comwrites:

Richard wrote:
>Richard<rg****@gmail.comwrites:

>>pete <pf*****@mindspring.comwrites:

Richard wrote:

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
Is that now defined? or, after a 14 hour day, am I talking rubbish here?
>
Can e not be decremented to a value "smaller" than s?
That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value
So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

Yes.

>or to make my question more clear - does this mean the entire program is
"undefined"?

Yes.

There is a special rule that allows
the calculation of the "one past" pointer value.

Which is?

Sep 22 '08 #25

Richard

Richard<rg****@gmail.comwrites:

pete <pf*****@mindspring.comwrites:

>Richard wrote:
>>Richard<rg****@gmail.comwrites:

pete <pf*****@mindspring.comwrites:

Richard wrote:
>
>char * my_strrev(char *s){
> char t;
> char *e = s+strlen(s);
> while(e-->s){
>Is that now defined? or, after a 14 hour day, am I talking rubbish here?
>>
>Can e not be decremented to a value "smaller" than s?
That operation would be undefined.
>
An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value
So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

Yes.

>>or to make my question more clear - does this mean the entire program is
"undefined"?

Yes.

There is a special rule that allows
the calculation of the "one past" pointer value.

Which is?

And which now makes me ask if this is UDB too:

unsigned int i=0;
while(i--){
}

While I would not normally do such things I know they exist all over the
place. Keeping in mind that the -- value is never actually used.

Sep 22 '08 #26

Richard<rg****@gmail.comwrites:

pete <pf*****@mindspring.comwrites:

[...]

>There is a special rule that allows
the calculation of the "one past" pointer value.

Which is?

Described in detail in C99 6.5.6p8.

Given:

int arr[5];
int *ptr = &arr[0];

ptr+0 is the address of the first element of the array.
ptr+4 is the address of the last element of the array.
ptr+5 points just past the last element. This address may legally be
computed, compared, etc., but it cannot be dereferenced (more
precisely, attempting to dereference it invokes UB).

Attempting to compute ptr+N, for N outside the range 0..5, invokes
undefined behavior, whether you subsequently attempt to dereference
the resulting pointer value or not.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Sep 23 '08 #27

Richard<rg****@gmail.comwrites:
[...]

And which now makes me ask if this is UDB too:

(UDB meaning Undefined Behavior; the more common abbreviation around
here is UB.)

unsigned int i=0;
while(i--){
}

While I would not normally do such things I know they exist all over the
place. Keeping in mind that the -- value is never actually used.

No, why would you think the behavior might be undefined? Decrementing
an unsigned object with the value 0 simply sets it to UINT_MAX.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Sep 23 '08 #28

Richard<rg****@gmail.comwrites:

Ben Bacarisse <be********@bsb.me.ukwrites:

>Richard<rg****@gmail.comwrites:
<snip>
>>Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

>>but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

Testing it would not reveal the issue.

Why wouldn't it? (and I acknowledge the correction btw).

Had I run this through a debugger with s as an empty string (I must
admit I had assumed s had to be non null and a non empty string but fair
enough point you raise) I would spot it in about 1 second when e is
calculated.

I don't see how, but maybe your debugger is clever. If you take the
view that a pointer is a number (it's not but you have suggested that
simplification here recently) e will be an address one less that s,
the while won't trigger and nothing will (correctly) be done. How can
testing show that such an e is invalid and can't even be constructed
let alone compared to s?

But correction (still no null check):
#include <stdio.h>
#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
t = *s;
*s++ = *e;
*e = t;
}
}

<snip>

Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

It can't be and your code still does so. You could write:

char *end = str + strlen(str);
while (end str) {
char tmp = *str;
*str++ = *--end;
*end = tmp;
}

although this swaps characters that don't need to be swapped. I think
CBFalconer's test against length being 1 is a good one:

size_t length = strlen(str);
if (length 1) {
char *end = str + length;
while (--end str) {
char tmp = *str;
*str++ = *end;
*end = tmp;
}
}

I've put declarations where I prefer and used my own pet names but it
is essentially the same code.

--
Ben.

Sep 23 '08 #29

Richard

Ben Bacarisse <be********@bsb.me.ukwrites:

Richard<rg****@gmail.comwrites:

>Ben Bacarisse <be********@bsb.me.ukwrites:

>>Richard<rg****@gmail.comwrites:
<snip>
Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

but it seems more concise and
readable to me. No I didnt test it, but I guess the idea should be self
evident.

Testing it would not reveal the issue.

Why wouldn't it? (and I acknowledge the correction btw).

Had I run this through a debugger with s as an empty string (I must
admit I had assumed s had to be non null and a non empty string but fair
enough point you raise) I would spot it in about 1 second when e is
calculated.

I don't see how, but maybe your debugger is clever. If you take the
view that a pointer is a number (it's not but you have suggested that
simplification here recently) e will be an address one less that s,

I pointed out that on every single system I have used (and this is one)
a pointer is indeed represented as a number of sorts in the
debugger.

In addition, since the pointers are char * the default is also to
display the strings they point to.

What I would have noticed in the debugger was that e was initialised to
something less than s in the case of empty strings. I also posted the
caveat that I (foolishly) did not consider empty strings and this was
indeed an error on my part. This I would have considered an error
despite me being far more liberal on UDB than many here and that being
something I know I can improve on.

the while won't trigger and nothing will (correctly) be done. How can
testing show that such an e is invalid and can't even be constructed
let alone compared to s?

>But correction (still no null check):
#include <stdio.h>
#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
t = *s;
*s++ = *e;
*e = t;
}
}
<snip>
>Is that now defined? or, after a 14 hour day, am I talking rubbish here?

Can e not be decremented to a value "smaller" than s?

It can't be and your code still does so. You could write:

char *end = str + strlen(str);
while (end str) {
char tmp = *str;
*str++ = *--end;
*end = tmp;
}

I think I did this in another reply. But did the "--" in the while.

I like the code above.

>
although this swaps characters that don't need to be swapped. I think
CBFalconer's test against length being 1 is a good one:

Personally I still dont see the point : the code above is concise.

>
size_t length = strlen(str);
if (length 1) {
char *end = str + length;
while (--end str) {
char tmp = *str;
*str++ = *end;
*end = tmp;
}
}

I've put declarations where I prefer and used my own pet names but it
is essentially the same code.

Sep 23 '08 #30

Ben Bacarisse wrote:

Richard<rg****@gmail.comwrites:

<snip>

>Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

It also has the fatal error (to me) of returning a char*. This
encourages the silly mistake of trying to print a string and its
reverse with:

printf("%s & %s\n", s, strrev(s));

In fact it is even worse than that - it fails to return the
declared object. My version returns strlen, which it needs to
evaluate to find the end of the string, and thus avoids the caller
needing to call it again. Also note that my code returns a NULL if
the passed string is NULL, and correctly handles strings of length
0 and 1.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 23 '08 #31

Ben Bacarisse wrote:

CBFalconer <cb********@yahoo.comwrites:

<snip code>

>This is so horrible and faulty that I have to attach the
following. Don't forget to #include <strings.h>.

I'd include <string.hinstead. I haven't seen strings.h for
decades.

A point. A palpable point. Thanks.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 23 '08 #32

lovecreatesbea...

On Sep 23, 7:22*am, pete <pfil...@mindspring.comwrote:

Richard wrote:
#include <string.h>

char * my_strrev(char *s){
* * *char t;
* * *char *e = s+strlen(s)-1;
* * *while(e>s){
* * * * t * *= *s;
* * * * *s++ = *e;
* * * * *e-- = t;
* * *}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. *No I didnt test it, but I guess the idea should be self
evident.

It's undefined when attempting to reverse a zero length string.

How to reverse a zero length string for it has only a single NULL
terminating character : ) The reverse function quits if this is the
case.

Sep 23 '08 #33

lovecreatesbea...

On Sep 23, 9:28*am, "lovecreatesbea...@gmail.com"
<lovecreatesbea...@gmail.comwrote:

On Sep 23, 7:22*am, pete <pfil...@mindspring.comwrote:

Richard wrote:
#include <string.h>

char * my_strrev(char *s){
* * *char t;
* * *char *e = s+strlen(s)-1;
* * *while(e>s){
* * * * t * *= *s;
* * * * *s++ = *e;
* * * * *e-- = t;
* * *}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. *No I didnt test it, but I guess the idea should beself
evident.

It's undefined when attempting to reverse a zero length string.

How to reverse a zero length string for it has only a single NULL
terminating character : ) The reverse function quits if this is the
case.

My version

char *reverse(char *p)
{
char *p1, *p2, ch;

for (p1 = p; *(p1 + 1); p1++) ;
for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}

Sep 23 '08 #34

CBFalconer <cb********@yahoo.comwrites:

Ben Bacarisse wrote:
>Richard<rg****@gmail.comwrites:

<snip>

>>Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

It also has the fatal error (to me) of returning a char*.

The fatal error is that is does *not* return a char * :-)

--
Ben.

Sep 23 '08 #35

"lovecreatesbea...@gmail.com" <lo***************@gmail.comwrites:

On Sep 23, 9:28Â*am, "lovecreatesbea...@gmail.com"
<lovecreatesbea...@gmail.comwrote:
>On Sep 23, 7:22Â*am, pete <pfil...@mindspring.comwrote:

Richard wrote:
#include <string.h>

char * my_strrev(char *s){
Â* Â* Â*char t;
Â* Â* Â*char *e = s+strlen(s)-1;
Â* Â* Â*while(e>s){
Â* Â* Â* Â* t Â* Â*= *s;
Â* Â* Â* Â* *s++ = *e;
Â* Â* Â* Â* *e-- = t;
Â* Â* Â*}
}

I'm sure it breaks some sort of ISO magic, but it seems more concise and
readable to me. Â*No I didnt test it, but I guess the idea should be self
evident.

It's undefined when attempting to reverse a zero length string.

How to reverse a zero length string for it has only a single NULL
terminating character : ) The reverse function quits if this is the
case.

Yes, but that is not the problem.

My version

char *reverse(char *p)
{
char *p1, *p2, ch;

for (p1 = p; *(p1 + 1); p1++) ;

If p points to an empty string this, too, is probably UB. It may not
be -- one would have to see the call, but you should not even risk
this. A call like: char test[10]; test[0] = 0; reverse(test); is bad
news!

for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}

--
Ben.

Sep 23 '08 #36

Richard

Ben Bacarisse <be********@bsb.me.ukwrites:

CBFalconer <cb********@yahoo.comwrites:

>Ben Bacarisse wrote:
>>Richard<rg****@gmail.comwrites:

<snip>

Much nicer IMO:

#include <string.h>

char * my_strrev(char *s){
char t;
char *e = s+strlen(s)-1;
while(e>s){
t = *s;
*s++ = *e;
*e-- = t;
}
}

I'm sure it breaks some sort of ISO magic,

Not "magic" but it has undefined behaviour on empty strings.

It also has the fatal error (to me) of returning a char*.

The fatal error is that is does *not* return a char * :-)

Please notice the name of the function before we get too carried away
here ...

And of course, me not returning anything ... :-; Another cut and paste
mistake .... No more me posting code untested (with the caveat included)
to "get the idea".

Sep 23 '08 #37

Richard

pete <pf*****@mindspring.comwrites:

Richard wrote:
>Richard<rg****@gmail.comwrites:

>>pete <pf*****@mindspring.comwrites:

Richard wrote:

char * my_strrev(char *s){
char t;
char *e = s+strlen(s);
while(e-->s){
Is that now defined? or, after a 14 hour day, am I talking rubbish here?
>
Can e not be decremented to a value "smaller" than s?
That operation would be undefined.

An object pointer can have only 4 different kinds of values:
1 Address of an object
2 One more than the address of an object
3 null pointer value
4 indeterminate value
So whats an indeterminate value in this case?

Since the "--" is done after the compare is this still "undefined
behaviour"?

Yes.

>or to make my question more clear - does this mean the entire program is
"undefined"?

Yes.

OK, I learnt something there. I have been too lazy with certain aspects
(and I'm not referring to the silly zero length error there) of my C
coding and I never, ever considered a line like

while(p-- s)
to have UDB risks. Probably one of the problems from associating C too
much with being a low level language designed to produce efficient code
tied to the underlying CPU HW and never really giving too much of a hoot
about platform independence.

To get me started on the path of righteousness can you point out where I
should learn/read that in the n1256.txt? As, frankly, I find the
standard document quite daunting and confusing.

Sep 23 '08 #38

"lovecreatesbea...@gmail.com" wrote:

pete <pfil...@mindspring.comwrote:

.... snip ...

>
>It's undefined when attempting to reverse a zero length string.

How to reverse a zero length string for it has only a single
NULL terminating character : ) The reverse function quits if
this is the case.

Not mine, which was posted earlier. It also considers a NULL
passed in as indicating an empty string, and returns 0 (for input
length).

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 23 '08 #39

"lovecreatesbea...@gmail.com" wrote:

>

.... snip ...

>
My version

char *reverse(char *p) {
char *p1, *p2, ch;

for (p1 = p; *(p1 + 1); p1++) ;
for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}

Much better. It still doesn't handle a NULL input, and has the
error of returning the string pointer rather than the string
length. Which is only an error in optimization, not coding.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 23 '08 #40

Ben Bacarisse wrote:

"lovecreatesbea...@gmail.com" <lo***************@gmail.comwrites:

.... snip ...

>
>My version

char *reverse(char *p) {
char *p1, *p2, ch;

for (p1 = p; *(p1 + 1); p1++) ;

If p points to an empty string this, too, is probably UB. It may
not be -- one would have to see the call, but you should not even
risk this. A call like: char test[10]; test[0] = 0; reverse(test);
is bad news!

I think that is OK. The above for does nothing except set p1 == p.

>
> for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}

and this one does nothing because p2 is not < p1. Both are at p.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.

Sep 23 '08 #41

Peter Nilsson

Richard<rgr...@gmail.comwrote:

>>An object pointer can have only 4 different kinds of
values:
1 * *Address of an object
2 * *One more than the address of an object
3 * *null pointer value
4 * *indeterminate value

5 Address of a function

OK, I learnt something there. I have been too lazy with
certain aspects (and I'm not referring to the silly zero
length error there) of my C coding and I never, ever
considered a line like

while(p-- s)

to have UDB risks. ...
<snip>
To get me started on the path of righteousness can
you point out where I should learn/read that in the
n1256.txt? As, frankly, I find the standard document
quite daunting and confusing.

Your path to righteousness begins with not slagging off
people who are prepared to do what you can't or won't.

That said, look at 6.5.6 (additive operators),
specifically p8.

"...If both the pointer operand and the result point
to elements of the same array object, or one past the
last element of the array object, the evaluation shall
not produce an overflow; otherwise, the behavior is
undefined."

--
Peter

Sep 23 '08 #42

arnuld

On Mon, 22 Sep 2008 18:22:25 -0400, CBFalconer wrote:

This is so horrible and faulty that I have to attach the
following. Don't forget to #include <strings.h>. Note that it
returns strlen.

/* ======================= */
/* reverse string in place */
size_t revstring(char *stg)
{
char *last, temp;
size_t lgh;

lgh = strlen(stg);
if (lgh 1) {
last = stg + lgh; /* points to '\0' */
while (last-- stg) {
temp = *stg; *stg++ = *last; *last = temp;
}
}
return lgh;
} /* revstring */

I have edited it a little bit to get pointer to pointer as function
argument and combined it with my get_single_word program and it works
little strange though. Also, your function is much easier to read IMHO:
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
enum { WORD_SIZE = 28 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE } ;
int get_single_word( char** );
size_t revstring( char** );
int main(void)
{
char *word;

while( (GSW_OK == get_single_word(&word)) && (*word) )
{
revstring( &word );
printf("Reversed: [ %s ]\n", word);
}

return 0;
}

/* reverse string in place */
size_t revstring(char **stg)
{
char *last, temp;
size_t lgh;

lgh = strlen(*stg);
if (lgh 1) {
last = *stg + lgh; /* points to '\0' */
while (last-- *stg)
{
temp = **stg;
**stg++ = *last;
*last = temp;
}
}
return lgh;
}

int get_single_word( char** ppc )
{
unsigned ele_num;
int ch;
size_t word_length, word_length_interval;
char* word_begin;
char* new_mem;

ele_num = 0;

word_length = WORD_SIZE;
word_length_interval = 2;

*ppc = malloc(word_length * sizeof(**ppc));
word_begin = *ppc;
if( NULL == ppc )
{
return GSW_ENOMEM;

}
while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* trailing whitespace */
}
if( EOF != ch )
{
*word_begin++ = ch;
ele_num = 1;
}
while( (EOF != (ch = getchar())) && (! isspace(ch)) )
{
if( (word_length - 1) == ele_num++ )
{
new_mem = realloc( *ppc, (word_length_interval * word_length * sizeof **ppc) );
*ppc = new_mem;

if( new_mem )
{
word_begin = new_mem + (word_begin - *ppc);
word_length *= word_length_interval;
*ppc = new_mem;
}
else
{
*word_begin = '\0';
return GSW_ENORESIZE;
}
}

*word_begin++ = ch;
}
*word_begin = '\0';

return GSW_OK;
}

=================== OUTPUT =====================
[arnuld@dune ztest]$ gcc -ansi -pedantic -Wall -Wextra reverse-string.c
[arnuld@dune ztest]$ ./a.out
Ben
Reversed: [ neB ]
CBFalconer
Reversed: [ rBFalconeC ]
comp.kang.c++
Reversed: [ +omp.kang.c+c ]
[arnuld@dune ztest]$

--
www.lispmachine.wordpress.com
my email is @ the above blog.
Google Groups is Blocked. Reason: Excessive Spamming

Sep 23 '08 #43

Richard

Peter Nilsson <ai***@acay.com.auwrites:

Richard<rgr...@gmail.comwrote:

>>>An object pointer can have only 4 different kinds of
values:
1 Â* Â*Address of an object
2 Â* Â*One more than the address of an object
3 Â* Â*null pointer value
4 Â* Â*indeterminate value

5 Address of a function

>OK, I learnt something there. I have been too lazy with
certain aspects (and I'm not referring to the silly zero
length error there) of my C coding and I never, ever
considered a line like

while(p-- s)

to have UDB risks. ...
<snip>
To get me started on the path of righteousness can
you point out where I should learn/read that in the
n1256.txt? As, frankly, I find the standard document
quite daunting and confusing.

Your path to righteousness begins with not slagging off
people who are prepared to do what you can't or won't.

Hold on - I still dont hold with the other version from cbf. Frankly any
prodding he gets is his own fault - he spends 90% of his posts doing
much the same. And I dont slag off correct code either. I do however
slag off ridiculous bullying and silly preening with such claims as "I
dont need a debugger and neither do you" or "your debugger uses numbers
for pointers? Really"? etc etc etc . It makes this group a very tense
and strange place to be at times.

Anyway I appreciate the link.

>
That said, look at 6.5.6 (additive operators),
specifically p8.

"...If both the pointer operand and the result point
to elements of the same array object, or one past the
last element of the array object, the evaluation shall
not produce an overflow; otherwise, the behavior is
undefined."

Now back to "undefined".

And my code.

>while(p-- s)

Where does "undefined" in any shape or form indicate that there will be
knock on effects outside of the value of the variable in question
considering it is not used anymore? And if that variable is not used
anymore does it really matter? Its like using (or not using rather) an
uninitialised pointer.

Yes, I know "undefined" can mean "anything", but lets stay real here for
a moment. Possibly "undefined" is "defined" to mean "undefined for that
variable in question".

Or?

Sep 23 '08 #44