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

Discussion in 'C Programming' started by Gary, Aug 5, 2006.

  1. Gary

    Gary Guest

    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);
    }
    Gary, Aug 5, 2006
    #1
    1. Advertising

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

    --
    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)
    Richard Heathfield, Aug 5, 2006
    #2
    1. Advertising

  3. Gary

    Snis Pilbor Guest

    Gary wrote:
    > 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);
    }
    Snis Pilbor, Aug 6, 2006
    #3
  4. Snis Pilbor said:

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

    --
    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)
    Richard Heathfield, Aug 6, 2006
    #4
  5. 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);

    --

    Frederick Gotham
    Frederick Gotham, Aug 6, 2006
    #5
  6. 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.
    Ancient_Hacker, Aug 6, 2006
    #6
  7. Gary

    Richard Guest

    "Gary" <> writes:

    > 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.
    Richard, Aug 6, 2006
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. F. GEIGER
    Replies:
    3
    Views:
    756
    F. GEIGER
    Aug 6, 2004
  2. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,764
    Smokey Grindel
    Dec 2, 2006
  3. gbattine
    Replies:
    0
    Views:
    290
    gbattine
    Nov 1, 2006
  4. petersonrj
    Replies:
    0
    Views:
    117
    petersonrj
    Sep 17, 2004
  5. Dominick Baier
    Replies:
    2
    Views:
    185
    Patrick.O.Ige
    Oct 21, 2004
Loading...

Share This Page