String reversing problem

[Albert]

Why doesn't:

#include <stdio.h>

void reverse(char[], int);

main()
{
char s[5];

s[0] = 'h';
s[1] = 'e';
s[2] = 'l';
s[3] = 'l';
s[4] = 'o';
reverse(s, 5);

for (int i=0; i<=4; i++)
putchar(s[i]);
return 0;
}

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

for (i=0,j=num_elements-1; (i<=num_elements-1) && (j>=0); i++,j--)
s[i] = s[j];
}

output:

olleh

?

[Artie Gold]

Think what happens when i==4 and j==1, for example. [Do they still call
it `desk checking'?]

> s[i] = s[j];
> }
>
> output:
>
> olleh
>
> ?
>

HTH,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com (new post 8/5)
http://www.cafepress.com/goldsays
"If you have nothing to hide, you're not trying!"

[Diptendra]

use this one
#include <stdio.h>

void reverse(char[], int);

main()
{
char s[5];
int i;

s[0] = 'h';
s[1] = 'e';
s[2] = 'l';
s[3] = 'l';
s[4] = 'o';
reverse(s, 5);

for ( i = 0; i<=4; i++)

[Albert]

What do you mean by 'desk checking'?

[sleb...@gmail.com]

Albert wrote:
> What do you mean by 'desk checking'?

I think he means checking by pen & paper. I'd call it "checking by pen
& paper" although I always use a whiteboard for it.

Computers can only do what you tell them to do. As they say: garbage
in, garbage out. Sometimes when the computer doesn't do what you want
it is worth checking if you told it to do what you thought you wanted.

[sleb...@gmail.com]

Albert wrote:
> Why doesn't:
>

> <snip>

> s[i] = s[j];
>

This is a good example of why desk-checking is good. Lets take a
"hello" string shall we? Note that in the "diagram" below I mark the
variable being "read" from with + and the variable being "written" to
with ^.

h e l l o // original string

o e l l o // doing s[0] = s[4]
^ +

o l l l o // doing s[1] = s[3]
^ +

o l l l o // doing s[2] = s[2]
^

o l l l o // doing s[3] = s[1]
+ ^

At this point I hope you see the problem with you code since you've
overwritten the original 'e' with an 'l'.

[pai]

hi ,
I think an extra variable is needed to store, bcoz u cant
interchange 2 variable as such.
or is there any way to do it ...
Pai

[Richard Heathfield]

pai said:

There is a way to do this under certain conditions, but it's not a very
bright idea.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[Chuck F.]

pai wrote:
>
> I think an extra variable is needed to store, bcoz u cant
> interchange 2 variable as such. or is there any way to do it
>

Include context, without which your message is meaningless. For
means on the broken google interface, see my sig below.

Try this, after #include <string.h>:

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

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

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>

[haroon]

Albert wrote:
> Why doesn't:

[...]

> void reverse(char s[], int num_elements)
> {
> int i, j;
>
> for (i=0,j=num_elements-1; (i<=num_elements-1) && (j>=0); i++,j--)
> s[i] = s[j];
> }

try writing reverse(...) like this:

/***/

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

char t;

for (i=0,j=num_elements-1; (i<=(num_elements-1) / 2) && (j>=0);
i++,j--)
{
t = s[i];

s[i] = s[j];

s[j] = t;
}
}

/***/

then analyze both to figure out whats the difference and what happend.

[tmp123]

Hi,

I agree on the previous, and, of course, if some day I need to code
something similar I will write more or less the same (specially in an
answer to a beginner).

But, just for fun, and taken into account is the third time this
question has been posted, another version:

int revstring ( char *s )
{
char *e;
int r;
for( e=s+(r=strlen(s))-1; s<e; *s^=*e^=*s^=*e, s++, e--);
return r;
}

Kind regards.

(hope google doesn't heats indentation).

[William J. Leary Jr.]

"Albert" <albert.xt...@gmail.com> wrote in message
news:1135915021....@g43g2000cwa.googlegroups.com...

> What do you mean by 'desk checking'?

I'd have called it "paper check," or, when I'm feeling whimsical "let's play
computer."

Whatever way it's expressed, it means get out some paper and a pen(cil) and
perform, yourself, the steps the computer will take to execute your program.
Sometimes it's better (or quicker) than a debugger. Sometimes it's the only
way to debug (for very limited platforms, for example). Well worth practicing.

- Bill

[Chuck F.]

tmp123 wrote:
> Chuck F. wrote:
>
... snip ...

>
>> /* reverse string in place. Return length */
>> static size_t revstring(char *stg)
>> {
>> char *last, temp;
>> size_t lgh;
>>
>> if ((lgh = strlen(stg)) > 1) {
>> last = stg + lgh; /* points to '\0' */
>> while (last-- > stg) {
>> temp = *stg; *stg++ = *last; *last = temp;
>> }
>> }
>> return lgh;
>> } /* revstring */
>

... snip ...

>
> But, just for fun, and taken into account is the third time this
> question has been posted, another version:
>
> int revstring ( char *s )
> {
> char *e;
> int r;
> for( e=s+(r=strlen(s))-1; s<e; *s^=*e^=*s^=*e, s++, e--);
> return r;
> }

FYI your version invokes undefined behaviour. Try it with a string
of zero chars. My length test is not there for fun. I also have
evil suspicions about the xor operations.

[tmp123]

Chuck F. wrote:

> tmp123 wrote:
> > int revstring ( char *s )
> > {
> > char *e;
> > int r;
> > for( e=s+(r=strlen(s))-1; s<e; *s^=*e^=*s^=*e, s++, e--);
> > return r;
> > }
>
> FYI your version invokes undefined behaviour. Try it with a string
> of zero chars. My length test is not there for fun. I also have
> evil suspicions about the xor operations.
>

Hi,

If length is 0, then e=s-1, thus s<e is false exiting loop.

However, if the usage of pointer comparation and pointer substraction
is not welcome, another version:

void revstring ( char *s )
{
char *e;
for( e=s+strlen(s); s!=e-- && s!=e; *s^=*e^=*s++^=*e);
}

Kind regards.

PS: some days ago, someone posted about if "C is easy". I do not known,
but it is tricky.

[Keith Thompson]

"tmp123" <tmp...@menta.net> writes:
[...]

> void revstring ( char *s )
> {
> char *e;
> for( e=s+strlen(s); s!=e-- && s!=e; *s^=*e^=*s++^=*e);
> }

Undefined behavior.

> PS: some days ago, someone posted about if "C is easy". I do not known,
> but it is tricky.

It can be if you go out of your way to make it tricky.

--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[tmp123]

Keith Thompson wrote:
> "tmp123" <tmp...@menta.net> writes:
> [...]
> > void revstring ( char *s )
> > {
> > char *e;
> > for( e=s+strlen(s); s!=e-- && s!=e; *s^=*e^=*s++^=*e);
> > }
>
> Undefined behavior.
>

Could be... but, could you prove your statement?

[Tim Rentsch]

"tmp123" <tmp...@menta.net> writes:

Even the first half of the control expression (namely, 's!=e--')
can yield undefined behavior if strlen(s) == 0.

[Keith Thompson]

The expression
*s^=*e^=*s++^=*e
modifies both *s and *e twice between sequence points.

[Christian Bau]

In article <1135977804.8...@g47g2000cwa.googlegroups.com>,
"tmp123" <tmp...@menta.net> wrote:

It's kind of obvious. Even if it wasn't, whoever wrote that kind of code
should be slapped silly. Immediate removal from any programming team
that I am involved with.

[Chuck F.]

tmp123 wrote:
> Chuck F. wrote:
>> tmp123 wrote:
>> > int revstring ( char *s )
>> > {
>> > char *e;
>> > int r;
>> > for( e=s+(r=strlen(s))-1; s<e; *s^=*e^=*s^=*e, s++, e--);
>> > return r;
>> > }
>>
>> FYI your version invokes undefined behaviour. Try it with a
>> string of zero chars. My length test is not there for fun. I
>> also have evil suspicions about the xor operations.
>

> If length is 0, then e=s-1, thus s<e is false exiting loop.
>
> However, if the usage of pointer comparation and pointer
> substraction is not welcome, another version:

> void revstring ( char *s )
> {
> char *e;
> for( e=s+strlen(s); s!=e-- && s!=e; *s^=*e^=*s++^=*e);
> }

Same problem and undefined behaviour. You are not allowed to
generate a pointer that points before s (although you are allowed
to generate one that points just after s).

[tmp123]

To Mr. Keith Thompson and Mr. Tim Rentsch:

Thanks for your replies, always open to learn something new.
The first comment mades me doubt: if strlen(s)==0, s==e, thus s!=e-- is
false and exit loop.
The second comments, about sequence points, well, I must recognize I do
not know what do you refer as "sequence points". An explanation or a
reference will be welcome.

To Mr. Christian Bau:

Your English seems not to be the most valid to be used in net, because
could be easily confused with agressive, specially by non-English
people. Moreover, it could be taken as a confusion between what is a
medium to interchange knowledgment, and what is a real programming
team.

Kind regards.

[Tim Rentsch]

"tmp123" <tmp...@menta.net> writes:

> To Mr. Keith Thompson and Mr. Tim Rentsch:
>
> Thanks for your replies, always open to learn something new.
> The first comment mades me doubt: if strlen(s)==0, s==e, thus s!=e-- is
> false and exit loop.

The problem is that the variable 'e' is decremented even if it is
equal to the address held in 's'. That means the decrement can
attempt to set 'e' to an address "before" the first element of
the array object holding the string, which is disallowed. It's
allowed to point to one element past the end of an array object,
but not allowed to point to one element before the beginning.

[Christian Bau]

In article <1135980646.5...@g49g2000cwa.googlegroups.com>,
"tmp123" <tmp...@menta.net> wrote:

Nothing wrong with my english. Lots wrong with your code. I don't care
too much about the undefined behavior, because you managed to write
completely incomprehensible code for a very simple task.

void reverse_string (char* s)
{
int i = 0;
int j = strlen (s) - 1;

while (i < j)
{
char tmp = s [i];

s [i] = s [j];

s [j] = tmp;
++i;
--j;
}
}

works and is easy to understand.

[tmp123]

Thanks for the explanation.

I do not know any compiler nor OS with problems for this pointer (it is
only assigned, not used, and probably the final address will be valid).

However, if you say that standard allows to pass the end of the array
but not point before, I can accept the reasoning.

Kind regards.

[Mark McIntyre]

On 30 Dec 2005 14:10:46 -0800, in comp.lang.c , "tmp123"
<tmp...@menta.net> wrote:

> I do
>not know what do you refer as "sequence points". An explanation or a
>reference will be welcome.

©ISO/IEC ISO/IEC 9899:1999 (E)
Annex C
(informative)
Sequence points
1 The following are the sequence points described in 5.1.2.3:
— The call to a function, after the arguments have been evaluated
(6.5.2.2).
— The end of the first operand of the following operators: logical AND
&& (6.5.13);
logical OR || (6.5.14); conditional ? (6.5.15); comma , (6.5.17).
— The end of a full declarator: declarators (6.7.5);
— The end of a full expression: an initializer (6.7.8); the expression
in an expression
statement (6.8.3); the controlling expression of a selection statement
(if or switch)
(6.8.4); the controlling expression of a while or do statement
(6.8.5); each of the
expressions of a for statement (6.8.5.3); the expression in a return
statement
(6.8.6.4).
— Immediately before a library function returns (7.1.4).
— After the actions associated with each formatted input/output
function conversion
specifier (7.19.6, 7.24.2).
— Immediately before and immediately after each call to a comparison
function, and
also between any call to a comparison function and any movement of the
objects
passed as arguments to that call (7.20.5).

(Of Christian's comment)

>Your English seems not to be the most valid to be used in net,

Grow a thicker skin.

Personally I also thought the code was an abhomination and had I
discovered it in a project I was running I'd have told the programmer
to remove it forthwith.

Mark McIntyre
--

----== Posted via Newsfeeds.Com - Unlimited-Unrestricted-Secure Usenet News==----
http://www.newsfeeds.com The #1 Newsgroup Service in the World! 120,000+ Newsgroups
----= East and West-Coast Server Farms - Total Privacy via Encryption =----

[Mark McIntyre]

On 30 Dec 2005 15:04:08 -0800, in comp.lang.c , "tmp123"
<tmp...@menta.net> wrote:

>Thanks for the explanation.
>
>I do not know any compiler nor OS with problems for this pointer

Thats not relevant. If it breaks the rules of the C standard, then its
not guaranteed to work, and you should not do it. One day your code
will be run on an OS which does care, and you will be fired / lose
money / lose face or whatever because of your code error.

[Tim Rentsch]

"tmp123" <tmp...@menta.net> writes:

> Thanks for the explanation.

You're welcome, glad it was of help.

> I do not know any compiler nor OS with problems for this pointer (it is
> only assigned, not used, and probably the final address will be valid).

In most practical cases it won't be a problem. However, the Standard
is quite unambiguous that it is potentially a problem, and there are
some implementations (I'm pretty sure) where it fails.

> However, if you say that standard allows to pass the end of the array
> but not point before, I can accept the reasoning.

One way to learn about these things is to get a copy of the
Standard, and read it yourself. There's an updated version
you can get just by downloading:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf

If you get a copy then you can read up on sequence points or
whatever else (including array indexing and pointer arithmetic)
and many of the comments on comp.lang.c will make a lot more
sense.

> Kind regards.

Likewise. And Happy New Year.

[Richard Heathfield]

tmp123 said:

> Thanks for the explanation.
>
> I do not know any compiler nor OS with problems for this pointer

The backroom boys are working on one right now, for all you know. And it
might just be tomorrow's sensational new toy for other, unrelated reasons.
And your boss might just say, "let's migrate all our code to this new
thing", as bosses often do. And at that point, the guys who stuck to the
rules will have working code, and the guys who didn't, won't.

So it pays to do things properly.

[tmp123]

Hi,

See inlines:

Christian Bau wrote:

> tmp123 wrote:
> > To Mr. Christian Bau:
> >
> > Your English seems not to be the most valid to be used in net, because
> > could be easily confused with agressive, specially by non-English
> > people. Moreover, it could be taken as a confusion between what is a
> > medium to interchange knowledgment, and what is a real programming
> > team.
>
> Nothing wrong with my english. Lots wrong with your code. I don't care
> too much about the undefined behavior, because you managed to write
> completely incomprehensible code for a very simple task.

Code that has been clearly state as "just for fun", that is, as an
academic experiment. At least, me, I get new knoledgment about pointer
before arrays and sequence points. Thanks to persons who have provided
it.

And taken into account that this piece of code has been presented as
"the good one" in a real programming team, some comments about:

>
> void reverse_string (char* s)
> {
> int i = 0;
> int j = strlen (s) - 1;

1) It is not the same initialize a variable as give a variable the
first value it will take.
2) Not always a stack to add variables is available, or to add more
variables to it. Sometimes only modify code is allowed (i.e: patching
firmware in real time systems without stop them).
3) Relation between names "i" and "j" and their meaning/usage is
totally lost.

>
> while (i < j)

4) Never heard about "for" statement?. It is used to enclose in an easy
to read statement all control of the loop iterators (initializations,
exit condition and state update).

> {
> char tmp = s [i];

5) Declare char here, far of the semantically parent of it (char *s) it
is only a way to hide things. And lots of compilers will ignore it (no
new frame).

> s [i] = s [j];
> s [j] = tmp;

6) It seems this code must always be compiled with latest version of
advanced compilers. The responsability to convert array index
calculations to pointer operations, even integers to pointers, is
transferred to compiler.

> ++i;
> --j;

7) Lost lines here?

> }
> }
>
> works and is easy to understand.

8) "Works" is not a measure of quality.

This is my last post in this subject. I don not like to be troll, nor
feed trolls.

Kind regards.

[sleb...@gmail.com]

tmp123 wrote:
> Hi,
>
> See inlines:
>
> Christian Bau wrote:
> > tmp123 wrote:
> > > To Mr. Christian Bau:
> > >
> > > Your English seems not to be the most valid to be used in net, because
> > > could be easily confused with agressive, specially by non-English
> > > people. Moreover, it could be taken as a confusion between what is a
> > > medium to interchange knowledgment, and what is a real programming
> > > team.
> >
> > Nothing wrong with my english. Lots wrong with your code. I don't care
> > too much about the undefined behavior, because you managed to write
> > completely incomprehensible code for a very simple task.
>
> Code that has been clearly state as "just for fun", that is, as an
> academic experiment. At least, me, I get new knoledgment about pointer
> before arrays and sequence points. Thanks to persons who have provided
> it.

"For fun" still doesn't make your code correct. You stated that it
works but the experts here have pointed out that even so it is still
not correct "C". So that doesn't invalidate Mark's comments - your code
still is horrible, learn and move on.

As for your comment about Mark's tone being aggressive, you have to
learn that this is Usenet, and taking on an aggressive tone is a
tradition of the net long before you showed up. Some groups liks
comp.lang.tcl may be less aggressive but comp.lang.c is aggressive (I
learned that the hard way). Maybe it's because so many people keep
asking the same stupid questions here over and over again (and keep
making the same stupid mistakes that others have made before).

> > void reverse_string (char* s)
> > {
> > int i = 0;
> > int j = strlen (s) - 1;
>
> 1) It is not the same initialize a variable as give a variable the
> first value it will take.

What are you trying to say here? That code is correct.

> 2) Not always a stack to add variables is available, or to add more
> variables to it. Sometimes only modify code is allowed (i.e: patching
> firmware in real time systems without stop them).

I don't see how this is different from your code since you yourself
introduce the variable 'r'.

> 3) Relation between names "i" and "j" and their meaning/usage is
> totally lost.

Lost? I understood it. The code is short and clear so it is
stylistically correct to use simple variable names (n, x, y, i, j
etc..). For that matter, the "relation between" 'r' and 's' in your
code and "their meaning/usage" also fall into the same category.

> > while (i < j)
>
> 4) Never heard about "for" statement?. It is used to enclose in an easy
> to read statement all control of the loop iterators (initializations,
> exit condition and state update).

"Easy to read"? Certainly not your code. And in Mark's code "while" is
quite natural - use the right tool for the right job.

> > {
> > char tmp = s [i];
>
> 5) Declare char here, far of the semantically parent of it (char *s) it
> is only a way to hide things. And lots of compilers will ignore it (no
> new frame).

This is correct and valid in C99 (unlike your code which is incorrect
and invalid in any C standard). Just because there are no C99 compilers
around doesn't mean that this code is not "C".

> > s [i] = s [j];
> > s [j] = tmp;
>
> 6) It seems this code must always be compiled with latest version of
> advanced compilers. The responsability to convert array index
> calculations to pointer operations, even integers to pointers, is
> transferred to compiler.

Nope, the above fragment of code will compile on any C compiler, I
suspect it is even valid in the original K&R "C" but I'm not sure.

> > works and is easy to understand.
>
> 8) "Works" is not a measure of quality.

True, as the experts here have pointed out about YOUR code. But "easy
to understand" IS a measure of quality.

> This is my last post in this subject. I don not like to be troll, nor
> feed trolls.

Mark have not been acting like a troll. He was merely scolding you for
writing poor code. You however are increasingly acting in a troll like
manner by insisting to argue even when you've been proven wrong.

[sleb...@gmail.com]

Oh crap, I thought tmp123 was referring to Mark. I meant Christian.
Sorry for the identity mix-up. My comments still stand though.

[Flash Gordon]

tmp123 wrote:
> Hi,
>
> See inlines:
>
> Christian Bau wrote:
>> tmp123 wrote:
>>> To Mr. Christian Bau:
>>>
>>> Your English seems not to be the most valid to be used in net, because
>>> could be easily confused with agressive, specially by non-English
>>> people. Moreover, it could be taken as a confusion between what is a
>>> medium to interchange knowledgment, and what is a real programming
>>> team.
>> Nothing wrong with my english. Lots wrong with your code. I don't care
>> too much about the undefined behavior, because you managed to write
>> completely incomprehensible code for a very simple task.
>
> Code that has been clearly state as "just for fun", that is, as an
> academic experiment. At least, me, I get new knoledgment about pointer
> before arrays and sequence points. Thanks to persons who have provided
> it.

If it is a learning exercise, then you should appreciate the advice not
dismiss it because it is only a learning exercise.

> And taken into account that this piece of code has been presented as
> "the good one" in a real programming team, some comments about:
>
>> void reverse_string (char* s)
>> {
>> int i = 0;
>> int j = strlen (s) - 1;
>
> 1) It is not the same initialize a variable as give a variable the
> first value it will take.

What you are saying makes no sense. Initialising a variable *is* giving
it the first value it will take!

> 2) Not always a stack to add variables is available, or to add more
> variables to it. Sometimes only modify code is allowed (i.e: patching
> firmware in real time systems without stop them).

What has that got to do with writing the code properly first time?

> 3) Relation between names "i" and "j" and their meaning/usage is
> totally lost.

i and j are conventionally used as loop iterators. You will just have to
get used to it if you are going to read other peoples code.

>> while (i < j)
>
> 4) Never heard about "for" statement?. It is used to enclose in an easy
> to read statement all control of the loop iterators (initializations,
> exit condition and state update).

I'm sure he has, but there is nothing intrinsically wrong with what
Christian Bau has done. In a case like this I'm in two minds about
whether to do it the way he has done it or using a for loop and not
initialising the variables on declaration since I can see arguments both
ways.

>> {
>> char tmp = s [i];
>
> 5) Declare char here, far of the semantically parent of it (char *s) it
> is only a way to hide things. And lots of compilers will ignore it (no
> new frame).

It is called localising scope and is often considered a good thing.
Whether or not the compiler generates a new stack frame (which is
unlikely) is irrelevant.

>> s [i] = s [j];
>> s [j] = tmp;
>
> 6) It seems this code must always be compiled with latest version of
> advanced compilers. The responsability to convert array index
> calculations to pointer operations, even integers to pointers, is
> transferred to compiler.

Compilers have been doing this king of optimisation for over 10 years,
so I hardly think a modern compiler is required.

>> ++i;
>> --j;
>
> 7) Lost lines here?

What lost lines? I see nothing missing.

>> }
>> }
>>
>> works and is easy to understand.
>
> 8) "Works" is not a measure of quality.

So you consider a program that does not work to be of the same quality
as one that does work? That is just plain stupid. His code is of far
higher quality by all measures I would use than your code is.

> This is my last post in this subject. I don not like to be troll, nor
> feed trolls.

Well, Christian Bau is not a troll. He posted a good example of how to
implement the function in order to assist you. If you cannot see that
then that is your problem.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Flash Gordon]

sleb...@yahoo.com wrote:
> tmp123 wrote:
>> Hi,
>>
>> See inlines:
>>
>> Christian Bau wrote:
>>> tmp123 wrote:
>>>> To Mr. Christian Bau:

<snip>

>>> {
>>> char tmp = s [i];
>> 5) Declare char here, far of the semantically parent of it (char *s) it
>> is only a way to hide things. And lots of compilers will ignore it (no
>> new frame).
>
> This is correct and valid in C99 (unlike your code which is incorrect
> and invalid in any C standard). Just because there are no C99 compilers
> around doesn't mean that this code is not "C".

Actually, you declaring a variable at the start of any block is valid
for all versions of C. Also, there are I believe a few complete C99
implementations, just not many.

>>> s [i] = s [j];
>>> s [j] = tmp;
>> 6) It seems this code must always be compiled with latest version of
>> advanced compilers. The responsability to convert array index
>> calculations to pointer operations, even integers to pointers, is
>> transferred to compiler.
>
> Nope, the above fragment of code will compile on any C compiler, I
> suspect it is even valid in the original K&R "C" but I'm not sure.

It is valid for all versions.

>>> works and is easy to understand.
>> 8) "Works" is not a measure of quality.
>
> True, as the experts here have pointed out about YOUR code. But "easy
> to understand" IS a measure of quality.

<snip>

Whether code works is definitely one of the measures of quality, just
not the only one. If code does not work it is poor quality even if it is
easy to read. For example, here is a very low quality, but easy to read,
implementation of a program to copy standard input to standard output.
#include <stdio.h>
int main(FILE *standard_input, FILE *standard_output)
{
standard_output = standard_input;
}

Nice and simple, easy to read, compiles under gcc (with -ansi -pedantic)
without warning or error, but probably one of the lowest quality
programs I've ever seen. I could also do ones that are (by some measure)
just as low quality but do not invoke undefined behaviour.

[Emmanuel Delahaye]

Albert a écrit :

> void reverse(char s[], int num_elements)
> {
> int i, j;
>
> for (i=0,j=num_elements-1; (i<=num_elements-1) && (j>=0); i++,j--)

> s[i] = s[j];
> }

You need a swap action, hence a local variable.

--
A+

Emmanuel Delahaye

[sleb...@gmail.com]

Flash Gordon wrote:
> sleb...@yahoo.com wrote:
> > tmp123 wrote:
> >> Hi,
> >>
> >> See inlines:
> >>
> >> Christian Bau wrote:
> >>> tmp123 wrote:
> >>>> To Mr. Christian Bau:
>
> <snip>
>
> >>> {
> >>> char tmp = s [i];
> >> 5) Declare char here, far of the semantically parent of it (char *s) it
> >> is only a way to hide things. And lots of compilers will ignore it (no
> >> new frame).
> >
> > This is correct and valid in C99 (unlike your code which is incorrect
> > and invalid in any C standard). Just because there are no C99 compilers
> > around doesn't mean that this code is not "C".
>
> Actually, you declaring a variable at the start of any block is valid
> for all versions of C.

Oh wow, tested it and it does work. I've always thought that "start of
block" refers to the start of the function. Learn something new
everyday. This is indeed nice as localising scope is usually a "good
thing"(tm).

[Mark McIntyre]

On 31 Dec 2005 03:05:17 -0800, in comp.lang.c , "tmp123"
<tmp...@menta.net> wrote:

>Christian Bau wrote:
>> void reverse_string (char* s)
>> {
>> int i = 0;
>> int j = strlen (s) - 1;
>
>1) It is not the same initialize a variable as give a variable the
>first value it will take.

Thats highly garbled, but I' guessing you're complaining that
initialising is not the same as assigning a value. So what?

>2) Not always a stack to add variables is available,

irrelevant since your own example also introduced variables

>3) Relation between names "i" and "j" and their meaning/usage is
>totally lost.

Rubbish. And your own version had the same mysterious loop variables.
If you object to that, call them start and end or somthing.

>> while (i < j)
>
>4) Never heard about "for" statement?. It is used to enclose in an easy
>to read statement all control of the loop iterators (initializations,
>exit condition and state update).

You're objecting to a while loop ? You're either a knave or a fool.

>> {
>> char tmp = s [i];
>
>5) Declare char here, far of the semantically parent of it (char *s) it
>is only a way to hide things.

No, its a way to simplify the code.

>And lots of compilers will ignore it (no new frame).

Absolute rubbish.

>> s [i] = s [j];
>> s [j] = tmp;
>
>6) It seems this code must always be compiled with latest version of
>advanced compilers. The responsability to convert array index
>calculations to pointer operations, even integers to pointers, is
>transferred to compiler.

Rubbish. This is perfectly correct C, and always has been.

>7) Lost lines here?

No, it seems you simply don't understand the function.

>8) "Works" is not a measure of quality.

Its one measure. If it doesn't work, then its useless rubbish code,
no?

>This is my last post in this subject. I don not like to be troll,

Then stop behaving like one.

Happy New Year by the way.

[Netocrat]

On Sat, 31 Dec 2005 12:26:04 +0000, Flash Gordon wrote:
[a refutation of tmp123's objections to a reverse_string() implementation]

I agree with most of your response, but I want to take up a couple of
points. Most of tmp123's legitimate objections related to style, except
for his indexing comment. Reading between the lines, his objections seem
to result in a rewriting of the function to something like this:

void reverse_string_mod(char* s)
{
char tmp, *ps, *pe;

if (*s != '\0')
{
for (ps = s, pe = s + strlen (s) - 1; ps < pe; ps++, pe--) {
tmp = *ps;
*ps = *pe;
*pe = tmp;
}
}
}

Where Christian Bau's original function was:

void reverse_string (char* s)
{
int i = 0;
int j = strlen (s) - 1;

while (i < j)

{
char tmp = s [i];

s [i] = s [j];
s [j] = tmp;

++i;
--j;
}
}

Due to the need for the zero-length test, there is no reduction in
vertical space - this possibility is what I believe tmp123 was implying
could be achieved when he wrote "Lost lines here?" (it was an obscure
wording though and I may have misinterpreted it).

As you (Flash Gordon) wrote:
> In a case like this I'm in two minds about whether to do it the way he

> [Christian Bau] has done it or using a for loop and not initialising the

> variables on declaration since I can see arguments both ways.

Likewise; although the reduction in vertical space and the clustering of
all looping information in one spot seem good reasons to prefer the for
loop style.

Vertical space is something I've come to be conservative about in C code,
since I find that - all other things equal, and with the qualification
that occasional empty lines for semantic demarcation are useful - the less
vertical scrolling/eye-movement required, the faster I can take in the
code's meaning. For this reason I prefer a bracing style that doesn't
place starting braces on a new line, and that avoids braces where it's
possible to do so without otherwise reducing readability.

[re a claim that indexing requires the compiler to optimise compared to
pointer arithmetic]

> Compilers have been doing this king of optimisation for over 10 years,
> so I hardly think a modern compiler is required.

The equivalent pointer-arithmetic code is both a well-accepted idiom and
optimal at an abstract level (at abstract level the additional indexing
calculations are always performed), and for those reasons I find it
preferable.

The optimisations of modern compilers are often unpredictable - doing some
checking using gcc and icc on my machine, I find that the
pointer-arithmetic version is /generally/ slightly faster across
optimisation levels and string lengths, but not always.

For comment, a vertically-minimalist re-writing of the pointer-arithmetic
version of the function:

void reverse_string_min_vert(char *s) {
char tmp, *ps, *pe;

if (*s != '\0')
for (ps = s, pe = s + strlen(s) - 1; ps < pe; ps++, pe--)
tmp = *ps, *ps = *pe, *pe = tmp;
}

Pros: very little vertical eye movement required; one line per high-level
operation (test for non-zero length, loop over the string, swap a
character on each iteration)
Cons: wide horizontal spacing; a lot of code to digest on each line; the
omitted braces may reduce the code's readability for those who are used to
mandatory braces.

Would this pass review at your shop? Why/why not?

--
http://members.dodo.com.au/~netocrat

[Flash Gordon]

Netocrat wrote:
> On Sat, 31 Dec 2005 12:26:04 +0000, Flash Gordon wrote:
> [a refutation of tmp123's objections to a reverse_string() implementation]

<snip>

> Due to the need for the zero-length test, there is no reduction in
> vertical space - this possibility is what I believe tmp123 was implying
> could be achieved when he wrote "Lost lines here?" (it was an obscure
> wording though and I may have misinterpreted it).

Very obscure if that is what it meant..

> As you (Flash Gordon) wrote:

<snip>

> [re a claim that indexing requires the compiler to optimise compared to
> pointer arithmetic]
>> Compilers have been doing this king of optimisation for over 10 years,
>> so I hardly think a modern compiler is required.
>
> The equivalent pointer-arithmetic code is both a well-accepted idiom and
> optimal at an abstract level (at abstract level the additional indexing
> calculations are always performed), and for those reasons I find it
> preferable.
>
> The optimisations of modern compilers are often unpredictable - doing some
> checking using gcc and icc on my machine, I find that the
> pointer-arithmetic version is /generally/ slightly faster across
> optimisation levels and string lengths, but not always.

Yes, but:
1) It is only the highest optimisation levels (one for speed and one for
space) that count IMHO, and you've not specified if there is any
difference at that point.
2) Readability and maintainability are far more important.

For this, I might have used either indexing or pointers, and would be
unlikely to comment on the choice made at a review.

> For comment, a vertically-minimalist re-writing of the pointer-arithmetic
> version of the function:
>
> void reverse_string_min_vert(char *s) {
> char tmp, *ps, *pe;
>
> if (*s != '\0')
> for (ps = s, pe = s + strlen(s) - 1; ps < pe; ps++, pe--)
> tmp = *ps, *ps = *pe, *pe = tmp;
> }
>
> Pros: very little vertical eye movement required; one line per high-level
> operation (test for non-zero length, loop over the string, swap a
> character on each iteration)
> Cons: wide horizontal spacing; a lot of code to digest on each line; the
> omitted braces may reduce the code's readability for those who are used to
> mandatory braces.
>
> Would this pass review at your shop? Why/why not?

I would not reject it out of hand but for me there is too much occurring
on each line and therefore takes me longer to read.

[tmp123]

sleb...@yahoo.com wrote:
> >
> > >>> {
> > >>> char tmp = s [i];
> > >> 5) Declare char here, far of the semantically parent of it (char *s) it
> > >> is only a way to hide things. And lots of compilers will ignore it (no
> > >> new frame).
> > >
> > > This is correct and valid in C99 (unlike your code which is incorrect
> > > and invalid in any C standard). Just because there are no C99 compilers
> > > around doesn't mean that this code is not "C".
> >
> > Actually, you declaring a variable at the start of any block is valid
> > for all versions of C.
>
> Oh wow, tested it and it does work. I've always thought that "start of
> block" refers to the start of the function. Learn something new
> everyday. This is indeed nice as localising scope is usually a "good
> thing"(tm).

"works" only "more or less". Imagine you have a function with two big
locals.The first one is used at the start of function, the second one
at the end. You can thing this version saves stack space:

void test ( void )
{
{
char tmp1[10000];
... some code
}
{
char tmp2[10000];
... more code
}
}

However, if you display the address of tmp1 and tmp2, you will see that
lots of compilers converts it to:
void test ( void )
{
char tmp1[10000];
char tmp2[10000];
... some code
... more code
}

that is not the expected one. That doesn't means this resource must not
be used. But it is good to known what will happen.

Kind regards.

[sleb...@gmail.com]

At the assembly level maybe but at the "C" level not true. Try
compiling:

#include <stdio.h>
int main()
{
int i;
for (i=0;i<10;i++) {
int n = i * 2;
printf("%d\n", n);
}
n = 3;
printf("%d\n", n);
}

and you'll get:

testprog.c: In function `main':
testprog.c:14: error: `n' undeclared (first use in this function)
testprog.c:14: error: (Each undeclared identifier is reported only once
testprog.c:14: error: for each function it appears in.)

Localising scope has little to do with trying to save memory but have
more to do with protecting variables from being misused. Think of the
difference of local and global, only in this case we get to use
variables that are more 'local' than local (if you know what I mean).

The first language I encountered this feature is in Perl. Little did I
know that C had it all along.

[tmp123]

Hi,

See inlines

Kind regards.

sleb...@yahoo.com wrote:

Thanks for showing the difference between variable scope, visibility,
life... . It can be interesting for some readers.

>
> Localising scope has little to do with trying to save memory but have
> more to do with protecting variables from being misused. Think of the
> difference of local and global, only in this case we get to use
> variables that are more 'local' than local (if you know what I mean).
>

See this function:

/* swap pairs of characters in strings: 12345 => 21435 */
void reverse ( char *s )
{
/* end condition */
if ( s[0]=='\0' || s[1]=='\0' ) return;

/* swap values */
{
char tmp;
tmp=s[0];
s[0]=s[1];
s[1]=tmp;
}

reverse(s+2);
}

> The first language I encountered this feature is in Perl. Little did I
> know that C had it all along.

There are older languages than perl with it.

Kind regards.

[Emmanuel Delahaye]

tmp123 a écrit :

> And taken into account that this piece of code has been presented as
> "the good one" in a real programming team, some comments about:
>
>
>>void reverse_string (char* s)
>>{
>> int i = 0;
>> int j = strlen (s) - 1;
>
>
> 1) It is not the same initialize a variable as give a variable the
> first value it will take.

The effect is similar. What exactly is your point ?

> 2) Not always a stack to add variables is available,

Correct, but a C implementation requires automatic memory space for the
purpose. If there is no automatic memory available, the implementation
is not compliant.

> or to add more
> variables to it. Sometimes only modify code is allowed (i.e: patching
> firmware in real time systems without stop them).

Sounds to be a twisted way of thinking...

> 3) Relation between names "i" and "j" and their meaning/usage is
> totally lost.

Ok, I could have used ir and iw instead (standing for read/write indexes).

>
>> while (i < j)
>
>
> 4) Never heard about "for" statement?. It is used to enclose in an easy
> to read statement all control of the loop iterators (initializations,
> exit condition and state update).

It's debatable. I personally prefer to use the for() constructs for
'canonic' loops

for (i = 0; i < n; i++)

For other usages (like the one here), I find while() more appropriate,
specially at debug stage.

>> {
>> char tmp = s [i];
>
>
> 5) Declare char here, far of the semantically parent of it (char *s) it
> is only a way to hide things. And lots of compilers will ignore it (no
> new frame).

I failed to understand your point. What exactly is wrong here ? All I
could say is that I should have used int instead of char.

>> s [i] = s [j];
>> s [j] = tmp;
>
>
> 6) It seems this code must always be compiled with latest version of
> advanced compilers. The responsability to convert array index
> calculations to pointer operations, even integers to pointers, is
> transferred to compiler.

So what ? It seems that you are having hard time to find a real failiure
in Christian's code. Never mind, making a fool of yourself in public was
definitely your choice.

--
A+

Emmanuel Delahaye

[Emmanuel Delahaye]

tmp123 a écrit :

> "works" only "more or less". Imagine you have a function with two big
> locals.The first one is used at the start of function, the second one
> at the end. You can thing this version saves stack space:
>
> void test ( void )
> {
> {
> char tmp1[10000];
> ... some code
> }
> {
> char tmp2[10000];
> ... more code
> }
> }
>
> However, if you display the address of tmp1 and tmp2, you will see that
> lots of compilers converts it to:
> void test ( void )
> {
> char tmp1[10000];
> char tmp2[10000];
> ... some code
> ... more code
> }
>
> that is not the expected one. That doesn't means this resource must not
> be used. But it is good to known what will happen.

This is completely a compiler issue. The C language semantics allows the
optimization, but the implementors are free to optimize or not.

--
A+

Emmanuel Delahaye

[tmp123]

Emmanuel Delahaye wrote:
> tmp123 a écrit :
>>

[...]

> > 2) Not always a stack to add variables is available,
>
> Correct, but a C implementation requires automatic memory space for the
> purpose. If there is no automatic memory available, the implementation
> is not compliant.
>
> > or to add more
> > variables to it. Sometimes only modify code is allowed (i.e: patching
> > firmware in real time systems without stop them).
>
> Sounds to be a twisted way of thinking...

No, it sounds like a real situation. Imagine you have a lot of machines
controling some public service of your country. Imagine these machines
have one process with lots of threads, each one controlling one user
session. Now, ops, you find an error on code: two variables must be
swap.

What you do? Stop all country? Update a few code is easy, but better
not to change in a live system the stack structure. Thus, use a trick
like a^=b^... could be the difference between a big problem and a
critical problem.

Moreover, in the previous post there was a lot of person saying "to
swap you need a local variable". Well, like it is explained in the
first level of any good programming course, there are more options.

>
> > 3) Relation between names "i" and "j" and their meaning/usage is
> > totally lost.
>
> Ok, I could have used ir and iw instead (standing for read/write indexes).
>

They are perfectly logical names... taking into account we are talking
about a swap of data!

> >
> >> while (i < j)
> >
> >
> > 4) Never heard about "for" statement?. It is used to enclose in an easy
> > to read statement all control of the loop iterators (initializations,
> > exit condition and state update).
>
> It's debatable. I personally prefer to use the for() constructs for
> 'canonic' loops
>
> for (i = 0; i < n; i++)
>
> For other usages (like the one here), I find while() more appropriate,

It seems you know more than the authors of C. Why not remove the "for"
syntax and change it to "for <lv> = 1 TO <expr>". It is preferable for
you?.

> specially at debug stage.

And after debug you modify code?

>
>
> So what ? It seems that you are having hard time to find a real failiure
> in Christian's code. Never mind, making a fool of yourself in public was
> definitely your choice.

But never ignorant.

>
> --
> A+
>
> Emmanuel Delahaye

Kind regards.

[Flash Gordon]

tmp123 wrote:
> Emmanuel Delahaye wrote:
>> tmp123 a écrit :
>
> [...]
>
>>> 2) Not always a stack to add variables is available,
>> Correct, but a C implementation requires automatic memory space for the
>> purpose. If there is no automatic memory available, the implementation
>> is not compliant.
>>
>>> or to add more
>>> variables to it. Sometimes only modify code is allowed (i.e: patching
>>> firmware in real time systems without stop them).
>> Sounds to be a twisted way of thinking...
>
> No, it sounds like a real situation. Imagine you have a lot of machines
> controling some public service of your country. Imagine these machines
> have one process with lots of threads, each one controlling one user
> session. Now, ops, you find an error on code: two variables must be
> swap.

How often do you come across a bug where the correct action is just to
swap to variables? I've come across lots of bugs but can't think of a
time when it was a bug significant to require a live patch where that
would solve it.

> What you do? Stop all country? Update a few code is easy, but better
> not to change in a live system the stack structure. Thus, use a trick
> like a^=b^... could be the difference between a big problem and a
> critical problem.

So how are you going to actually change the code on this running system
without breaking it? Especially as it is highly unlikely to be in a
place where you have left a load of noops in that you can just
overwrite. If there are, then you can just read one, push it on the
stack, move the other, and pop the one you save off and write it back.
I've yet to come across any situation when that would not be acceptable.
Mind you, as I say, the chances of you being able to patch the code for
for that on a live system are negligible.

BTW, I have been involved in patching live systems, but it was where we
knew there was the possibility of needing to patch things and we could
design things in such a way as to specifically allow it. It also
required working at the assembler level rather than in C, and I can't
see how the tricks we applied could be used at the C level.

> Moreover, in the previous post there was a lot of person saying "to
> swap you need a local variable". Well, like it is explained in the
> first level of any good programming course, there are more options.

In assembler programming it might sometimes make sense to use another
method, I've yet to see an instance where it would have made sense in
any higher level language, including in C.

<snip>

>>>> while (i < j)
>>>
>>> 4) Never heard about "for" statement?. It is used to enclose in an easy
>>> to read statement all control of the loop iterators (initializations,
>>> exit condition and state update).
>> It's debatable. I personally prefer to use the for() constructs for
>> 'canonic' loops
>>
>> for (i = 0; i < n; i++)
>>
>> For other usages (like the one here), I find while() more appropriate,
>
> It seems you know more than the authors of C. Why not remove the "for"
> syntax and change it to "for <lv> = 1 TO <expr>". It is preferable for
> you?.

Because then you could not do a list traversal for loop possibly.
Anyway, since no one uses every facility of any language, it should
harldy be a surprise when someone says they would not use a particular
facility in some specific way.

>> specially at debug stage.
>
> And after debug you modify code?

I doubt he would.

>> So what ? It seems that you are having hard time to find a real failiure
>> in Christian's code. Never mind, making a fool of yourself in public was
>> definitely your choice.
>
> But never ignorant.

Actually, you do sound ignorant. Whether you are or not is a completely
different matter.

[Keith Thompson]

"tmp123" <tmp...@menta.net> writes:
> Emmanuel Delahaye wrote:
>> tmp123 a écrit :
[...]

>> > or to add more
>> > variables to it. Sometimes only modify code is allowed (i.e: patching
>> > firmware in real time systems without stop them).
>>
>> Sounds to be a twisted way of thinking...
>
> No, it sounds like a real situation. Imagine you have a lot of machines
> controling some public service of your country. Imagine these machines
> have one process with lots of threads, each one controlling one user
> session. Now, ops, you find an error on code: two variables must be
> swap.
>
> What you do? Stop all country? Update a few code is easy, but better
> not to change in a live system the stack structure. Thus, use a trick
> like a^=b^... could be the difference between a big problem and a
> critical problem.

I can imagine the need to patch live code under some circumstances,
but doing so is not C programming.

It's conceivable, I suppose, that in the process of doing so you might
need to use the xor trick to swap two variables, but it hardly seems
likely.

> Moreover, in the previous post there was a lot of person saying "to
> swap you need a local variable". Well, like it is explained in the
> first level of any good programming course, there are more options.

Sure, there are more options, but no better ones. The dangers of the
xor trick are explained in the C FAQ. Some CPUs may have swap
instructions, but there's no direct way to use them from C (though a
decent optimizing compiler should be able to generate one from the
obvious code).

The way to swap two variables in C is:
temp = x;
x = y;
y = x;
If you feel the need to use some other method, you should explain why
the obvious technique won't work for you.

[...]

>> So what ? It seems that you are having hard time to find a real failiure
>> in Christian's code. Never mind, making a fool of yourself in public was
>> definitely your choice.
>
> But never ignorant.

Never? I find that difficult to believe. My own areas of ignorance
are vast (though I try to acknowledge them); I would expect the same
of any finite being.

--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[pete]

Albert wrote:

> char s[5];
>
> s[0] = 'h';
> s[1] = 'e';
> s[2] = 'l';
> s[3] = 'l';
> s[4] = 'o';

That's not a string.

/* BEGIN new.c */

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

char *str_rev(char *s);

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

puts(str_rev(s));
return 0;
}

char *str_rev(char *s)
{
char *t, swap;
char *const p = s;

if (*s != '\0') {
t = s + strlen(s + 1);
while (t > s) {
swap = *t;
*t-- = *s;
*s++ = swap;
}
}
return p;
}

/* END new.c */

--
pete