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!

 

[Alexander Bartolich]

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

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

 

[kernelxu]

char* s = "Hello world!";

char s[] = "Hello world!";

will be OK.

 

[Cong Wang]

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!

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.

 

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

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>

 

[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:[email protected]>

 

[Flash Gordon]

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

What compiler are you using? For gcc you may need to use the
--writable-strings flag.
!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.

 

[Keith Thompson]

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.

 

[Mark]

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]

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.

I disagree. Personal style issue, nothing more.

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


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

 

[Martin Ambuhl]

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.