string copy. this one works, but why that one doesn't?

[Gary]

I'm confused about the two ways of string copy below. The first one
(two functions) works; the second doesn't. Can anybody explain the
reason to me? Thanks a lot.

#include <stdio.h>
/*this one works well
void copy (char *s, char *t)
{
int i = 0;

while ((*t++ = *s++) != '\0')
;
}

main ()
{
char *s = "abcde";
char *t;
copy (s, t);
printf ("%s\n", t);
}
*/


//put all of them together in main, why "Segmentation fault"?
main ()
{
char *s = "abcde";
char *t;

int i = 0;

while ((*t++ = *s++) != '\0')
;
printf ("%s\n", t);
}

 

[Richard Heathfield]

Gary said:

I'm confused about the two ways of string copy below. The first one
(two functions) works; the second doesn't. Can anybody explain the
reason to me? Thanks a lot.

In C, all arguments to a function are evaluated, and their values passed to
the function, which receives those values in its local parameter objects.
No matter how the function changes those local copies, the original
expressions evaluated in the call are not affected in the slightest.

#include <stdio.h>
/*this one works well
void copy (char *s, char *t)
{
int i = 0;

while ((*t++ = *s++) != '\0')
;

When this loop is complete, both t and s point to the end of the string, not
the beginning.

}

But now t and s have ceased to exist, so it hardly matters.

main ()
{
char *s = "abcde";
char *t;
copy (s, t);

This is an error. t doesn't point to any storage. Everything Has To Be
Somewhere. When you make a copy of s, that copy has to be put somewhere.
But you haven't specified anywhere to put it.

But if we pretend you'd written:

char buf[6];
char *t = buf;

that would point t at enough storage to hold a copy of "abcde".

printf ("%s\n", t);
}
*/


//put all of them together in main, why "Segmentation fault"?
main ()
{
char *s = "abcde";
char *t;

Let's fix this to:

char buf[6];
char *t = buf;

int i = 0;

while ((*t++ = *s++) != '\0')
;

Just like before, you're walking these pointers to the end of the string.
But this time, they don't magically vanish, leaving the originals as they
were before your copy() call. This time, they're /still/ at the end of the
string.

 

[Snis Pilbor]

I'm confused about the two ways of string copy below. The first one
(two functions) works; the second doesn't. Can anybody explain the
reason to me? Thanks a lot.

#include <stdio.h>
/*this one works well
void copy (char *s, char *t)
{
int i = 0;

while ((*t++ = *s++) != '\0')
;
}

main ()
{
char *s = "abcde";
char *t;
copy (s, t);
printf ("%s\n", t);
}
*/


//put all of them together in main, why "Segmentation fault"?
main ()
{
char *s = "abcde";
char *t;

int i = 0;

while ((*t++ = *s++) != '\0')
;
printf ("%s\n", t);
}

In addition to what Richard Heathfield already wrote, I think it should
be emphasized that the first method of copying is also broken. The
fact that it worked when you tried it was sheer blind luck and can't be
depended on. The reason is that t is uninitialized. When you declare
"char *t", you are telling the program: "temporarily give me a block
of memory big enough to hold the address of a char." The program gives
you that block, but you never say what address to store in that block.
Kind of like buying a cellphone at a yard sale, then immediately trying
to use it's speed dial to call your friend. A more correct way to do
the first version would be

#include <stdio.h>
void copy (char *s, char *t)
{
while ((*t++ = *s++) != '\0')
;
}

main ()
{
char *s = "abcde";
char t[6];

copy (s, t);
printf ("%s\n", t);
}

 

[Richard Heathfield]

Snis Pilbor said:

In addition to what Richard Heathfield already wrote, I think it should
be emphasized that the first method of copying is also broken. The
fact that it worked when you tried it was sheer blind luck and can't be
depended on. The reason is that t is uninitialized.

For the record, I did actually point this out in my earlier reply. But
you're right that it should be emphasised.

 

[Frederick Gotham]

Gary posted:

I'm confused about the two ways of string copy below. The first one
(two functions) works; the second doesn't. Can anybody explain the
reason to me? Thanks a lot.

#include <stdio.h>
/*this one works well
void copy (char *s, char *t)

It's customary to put the destination buffer first, and then the source
buffer next (this is how the C Standard Library does it).

Also, make use of "const":

void copy(char *dest, char const *source)

{
int i = 0;

What's that doing there? You never use it.

while ((*t++ = *s++) != '\0')
;
}

Your algorithm works OK.

main ()
{
char *s = "abcde";
char *t;

A floating-point type stores a floating-point number.

An integer type stores an integer number.

A pointer type stores a memory address.

If you don't initialise a local variable, then it contains garbage. In your
code snippet, you don't initialise "t" and so it contains a garbage memory
address. It may contain the memory address 0x2325ADD234, which points to
Operating System memory which you're not allowed play around with.

The solution would perhaps be something like the following:

char const source[] = "abcde";

char dest[sizeof source];

copy(dest,source);

 

[Ancient_Hacker]

In an ideal world, when you write a function, it's really nice if:

(1) The function checks its input arguments for plausibility.

(2) The function knows the size and validity of where it is supposed to
put its output.

(3) The function does something reasonable with invalid input.

-----

Now in the wild world of C, these things are not always knowable.

But you could at least check the input and output parameters to ensure
they're not NULL. That will catch many otherwise hard to find fatal
errors. In some C's, there are (non-standard) functions that can tell
you if a pointer is pointing to writeable memory, that can also be a
very helpful thing to try calling.

If you want to be really careful, and you know your strings are going
to be plain ASCII text, of reasonable length, you could even scan the
input string and not copy anything that has a character > 127 or a
length longer than some reasonable value you choose. That will catch a
lot of unset inputs.

And it's a really good idea to supply the length of the output place to
your copy function, so you don't go stomping all over memory. See
strncpy.

And as to what you do with invalid input, please don't silently return,
do something reasonable, like call some global error reporting
function.

 

[Richard]

I'm confused about the two ways of string copy below. The first one
(two functions) works; the second doesn't. Can anybody explain the
reason to me? Thanks a lot.

You got lucky/unlucky. You have undefined bahaviour because you have not
allocated space for "t" to store a string. In addition you have used
printf to print a string at the end of "t" in the "single function"
solution. See below.

#include <stdio.h>
/*this one works well
void copy (char *s, char *t)
{
int i = 0;

while ((*t++ = *s++) != '\0')
;
}

main ()
{
char *s = "abcde";
char *t;
copy (s, t);

"t" is not allocated memory to store a string.

printf ("%s\n", t);
}
*/


//put all of them together in main, why "Segmentation fault"?
main ()
{
char *s = "abcde";
char *t;

*oops*. You have declared a character pointer, but havent told the
program what it points to. Did your compiler not tell you this?

int i = 0;

while ((*t++ = *s++) != '\0')
;
printf ("%s\n", t);

You should probably do something like

char *p=t;
while(*p++=*s++);
printf("%s\n",t);

Also, what's "i" for? The rest you can figure out I guess. Use a
character array or malloc/free.