what's wrong with my dummy code?

[shya]

hi there!
I just wrote a function that reverse a string. It seems all ok to me,
but the program goes on segmentation fault on *s = *t_str.
Here's the code:

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

void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

int main(int argc, char *argv[])
{
char* s = "Hello world!";
reverse(s);
printf("%s", s);

return 0;
}

what's wrong??!
thanks!

--

Shya,

[Alexander Bartolich]

shya wrote:

[...] what's wrong??!

http://www.eskimo.com/~scs/C-faq/q1.32.html

--
post tenebras lux. post fenestras tux.

[kernelxu]

shya wrote:

char* s = "Hello world!";

char s[] = "Hello world!";

will be OK.

[Cong Wang]

shya wrote:

hi there!
I just wrote a function that reverse a string. It seems all ok to me,
but the program goes on segmentation fault on *s = *t_str.
Here's the code:

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

void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

int main(int argc, char *argv[])
{
char* s = "Hello world!";
reverse(s);
printf("%s", s);

return 0;
}

what's wrong??!
thanks!

--

Shya,

PS: Changing the arguments of a function isn't a good habit.

[pete]

Cong Wang wrote:

PS: Changing the arguments of a function isn't a good habit.

I prefer to alter function parameters at every given oportunity.

--
pete

[Trofinenco, Cristian [CAR:CW42:EXCH]]

shya wrote:

hi there!
I just wrote a function that reverse a string. It seems all ok to me,
but the program goes on segmentation fault on *s = *t_str.
Here's the code:

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

void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

int main(int argc, char *argv[])
{
char* s = "Hello world!";
reverse(s);
printf("%s", s);

return 0;
}

what's wrong??!
thanks!

What compiler are you using? For gcc you may need to use the
--writable-strings flag. gcc --writable-strings gg.c && ./a.out

!dlrow olleH> ~/tmp>

--
Cristian Trofinenco (CW42)
Tel.: [phone] (ESN 393-8674)
ct*****@nortel.com

[Flash Gordon]

Trofinenco, Cristian [CAR:CW42:EXCH] wrote:

<snip code that modifies a string literal>

What compiler are you using? For gcc you may need to use the
--writable-strings flag.

> gcc --writable-strings gg.c && ./a.out

!dlrow olleH> ~/tmp>

That is the *wrong* answer. Although it gets the expected result on your
version of gcc, it is NOT available on the latest version of gcc and
other compilers may well not have this feature. Also, this group only
deals with standard C, not extensions available on specific compilers.

The correct answer, one which I believe was given by someone else, is
*don't* modify string literals since the standard states that this is
undefined behaviour. Use an initialised character array instead.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Kenneth Brody]

Cong Wang wrote:
[...]

PS: Changing the arguments of a function isn't a good habit.

I guess you've never used sprintf, strcpy, fread, ...

--
+-------------------------+--------------------+-----------------------------+
| Kenneth J. Brody | www.hvcomputer.com | |
| kenbrody/at\spamcop.net | www.fptech.com | #include <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------------+
Don't e-mail me at: <mailto:Th*************@gmail.com>

[Keith Thompson]

Kenneth Brody <ke******@spamcop.net> writes:

Cong Wang wrote:
[...]

PS: Changing the arguments of a function isn't a good habit.

I guess you've never used sprintf, strcpy, fread, ...

Those functions don't (necessarily) change their parameters; they
change the objects that their parameters point to.

What was being called a bad habit was this kind of thing:

void foo(int x) {
{
x ++; /* changing the parameter */
}

There's nothing incorrect about doing this, but it can cause confusion
(though of course it won't affect the actual argument that was passed
to the function). Whether it's good or bad style in general is a
debate that I'm not going to get into, at least for now.

--
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.

[Mark]

"Keith Thompson" <ks***@mib.org> wrote in message
news:ln************@nuthaus.mib.org...

Kenneth Brody <ke******@spamcop.net> writes:

Cong Wang wrote:
[...]

PS: Changing the arguments of a function isn't a good habit.
I guess you've never used sprintf, strcpy, fread, ...

Those functions don't (necessarily) change their parameters; they
change the objects that their parameters point to.

Which is exactly what Cong complained about.
What was being called a bad habit was this kind of thing:

void foo(int x) {
{
x ++; /* changing the parameter */
} No, it wasn't that kind of thing.
There's nothing incorrect about doing this, but it can cause confusion I disagree. Personal style issue, nothing more.
(though of course it won't affect the actual argument that was passed
to the function). Whether it's good or bad style in general is a
debate that I'm not going to get into, at least for now. Good... because the actual code in question was as follows:
void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

Sometimes it is the PURPOSE of a function to change the object
that the parameters point to... Ken was merely trying to point that out.

Mark

[Keith Thompson]

"Mark" <so***@localbar.com> writes:

"Keith Thompson" <ks***@mib.org> wrote in message
news:ln************@nuthaus.mib.org...

Kenneth Brody <ke******@spamcop.net> writes:

Cong Wang wrote:
[...]
PS: Changing the arguments of a function isn't a good habit.

I guess you've never used sprintf, strcpy, fread, ...

Those functions don't (necessarily) change their parameters; they
change the objects that their parameters point to.

Which is exactly what Cong complained about.

What was being called a bad habit was this kind of thing:

void foo(int x) {
{
x ++; /* changing the parameter */
}

No, it wasn't that kind of thing.

Yes, it was; see below.

There's nothing incorrect about doing this, but it can cause confusion

I disagree. Personal style issue, nothing more.

(though of course it won't affect the actual argument that was passed
to the function). Whether it's good or bad style in general is a
debate that I'm not going to get into, at least for now.

Good... because the actual code in question was as follows:

void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

Sometimes it is the PURPOSE of a function to change the object
that the parameters point to... Ken was merely trying to point that out.

I think you missed it. The argument to reverse() is called s. The
function changes the value of s in the second for loop; see "s++".
(It also changes what s points to, but that's a different matter.)

--
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.

[Martin Ambuhl]

shya wrote:

hi there!
I just wrote a function that reverse a string. It seems all ok to me,
but the program goes on segmentation fault on *s = *t_str.
Do you still get one if instead of trying to reverse a string literal (a
bad thing) you try to reverse an array containing a string? Change char* s = "Hello world!"; to
char s[] = "Hello world!";

While you're at it, make your program portable by changing
printf("%s", s);

to
printf("%s\n", s);

Not writing an end-of-line character at the end of the last line of
output is not a good idea.

[Mark]

"Keith Thompson" <ks***@mib.org> wrote in message
news:ln************@nuthaus.mib.org...

"Mark" <so***@localbar.com> writes:

"Keith Thompson" <ks***@mib.org> wrote in message
news:ln************@nuthaus.mib.org...

Kenneth Brody <ke******@spamcop.net> writes:
Cong Wang wrote:
[...]
> PS: Changing the arguments of a function isn't a good habit.

I guess you've never used sprintf, strcpy, fread, ...

Those functions don't (necessarily) change their parameters; they
change the objects that their parameters point to.

Which is exactly what Cong complained about.

What was being called a bad habit was this kind of thing:

void foo(int x) {
{
x ++; /* changing the parameter */
}

No, it wasn't that kind of thing.

Yes, it was; see below.

There's nothing incorrect about doing this, but it can cause confusion

I disagree. Personal style issue, nothing more.

(though of course it won't affect the actual argument that was passed
to the function). Whether it's good or bad style in general is a
debate that I'm not going to get into, at least for now.

Good... because the actual code in question was as follows:

void reverse(char *s)
{
char t;
char* t_str;

for (t_str = s; *t_str != '\0'; t_str++);
t_str--;

for (; s < t_str; s++, t_str--) {
t = *s;
*s = *t_str;
*t_str = t;
}
}

Sometimes it is the PURPOSE of a function to change the object
that the parameters point to... Ken was merely trying to point that out.

I think you missed it. The argument to reverse() is called s. The
function changes the value of s in the second for loop; see "s++".
(It also changes what s points to, but that's a different matter.)

Yes, I missed it :-)
Thanks for pointing that out.

After re-reading the thread I see that rather than sprintf() strcpy()
and fread() Ken should have said strcat(), strcpy(), strcmp() !!!
Many implementations of those functions do 'change' their parameters,
and why not? Parameters belong to the function... they are theirs
to use and change as the function sees fit.

Now please allow me to take this thread in another direction...
(I think I already did ;-) I know you don't want to get into the
'style' issue, but why not?

To make a broad statement such as: changing the arguments of a
function isn't a good habit - is not a good habit. Depending on the
circumstances, it doesn't bother me in the least to see it done.

consider the following:

void
do_something(int max_times)
{
while(max_times--)
something();
return;
}

Looks fine to me... doesn't bother me in the least.
Personally, I prefer that to code which used a copy
of max_times for no other reason than to be anal.

I've seen the arguments before...
for: clean concise code.
against: can't determine initial value.
for: who gives a 5hit? I don't need the initial value.
against: something() may break, debugger lost the initial value.
for: go up a frame
.... [it goes on and on] ...
against: I once read on comp.lang.c that it's not a good habit.
for: *&$K them too :-)

Which side of the fence are you guys on? :-)

For those on the 'against' side: you may want to take a look
at your implementation's standard library functions, you may
have to rewrite some of them if it bothers you that much.

I look forward to reading all responses... on Monday!
Have a nice weekend guys :-)
Mark