Code Review...

Discussion in 'C Programming' started by Vijay Kumar R Zanvar, Dec 24, 2003.

  1. Hi,

    I invite reviews for the following code:

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

    int
    main ( void )
    {
    char *p;

    p = (char*) &p;
    strcpy ( p, "Hi" );
    printf ( "%s\n", p );
    return EXIT_SUCCESS;
    }


    Thanks.

    --
    Vijay Kumar R Zanvar
    My Home Page - http://www.geocities.com/vijoeyz/
     
    Vijay Kumar R Zanvar, Dec 24, 2003
    #1
    1. Advertising

  2. Vijay Kumar R Zanvar

    Jack Klein Guest

    On Wed, 24 Dec 2003 11:45:50 +0530, "Vijay Kumar R Zanvar"
    <> wrote in comp.lang.c:

    > Hi,
    >
    > I invite reviews for the following code:


    Your code invokes undefined behavior.

    >
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int
    > main ( void )
    > {
    > char *p;


    p is an uninitialized pointer to char.

    > p = (char*) &p;


    p now contains the its own address.

    > strcpy ( p, "Hi" );


    Now you overwrite p's contents with three characters, 'H', 'i', and
    '\0'. Immediate undefined behavior if sizeof (char *) is < 3, which
    is true on many 16-bit implementations.

    > printf ( "%s\n", p );


    Undefined behavior for sure, you have modified the value of p via an
    lvalue of character type. Accessing it as a pointer, or indeed as
    anything other than an array of character type, is now undefined
    behavior.

    Undefined behavior also because printf() will attempt to dereference
    p, which almost certainly no longer points to a string your program
    has the right to access.

    > return EXIT_SUCCESS;
    > }
    >
    >
    > Thanks.


    What did you actually think this silly nonsense would be good for?

    --
    Jack Klein
    Home: http://JK-Technology.Com
    FAQs for
    comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
    comp.lang.c++ http://www.parashift.com/c -faq-lite/
    alt.comp.lang.learn.c-c++ ftp://snurse-l.org/pub/acllc-c /faq
     
    Jack Klein, Dec 24, 2003
    #2
    1. Advertising

  3. Vijay Kumar R Zanvar

    Ronny Mandal Guest

    "Vijay Kumar R Zanvar" <> wrote in message
    news:bsbapi$bfn8a$-berlin.de...
    > Hi,
    >
    > I invite reviews for the following code:
    >
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>

    Includes ok.
    >
    > int
    > main ( void )
    > {
    > char *p;
    >
    > p = (char*) &p;

    Why cast the pointers address to the pointer? WHen you operate on pointers,
    *p will give you accessto what is stored at the pointers address.
    Similar to ordinary variables:

    int p=5

    printf ( "%d\n", p ); will yield 5

    eq

    int *p = 5;

    printf ( "%d\n", *p ); will yield 5 also

    printf ( "%d\n", p ); will yield the address in memory where p is stored.

    Doing this cast will as always compile correctly, but yield a seg. fault.

    > strcpy ( p, "Hi" );
    > printf ( "%s\n", p );
    > return EXIT_SUCCESS;


    Assuming that EXIT_SUCCESS is 0 (simply put in a 'define EXIT_SUCCESS 0')
    > }
    >
    >
    > Thanks.
    >
    > --
    > Vijay Kumar R Zanvar
    > My Home Page - http://www.geocities.com/vijoeyz/
    >
    >


    --

    I hope that this was nearby the answer you wished for.

    Ronny Mandal
     
    Ronny Mandal, Dec 24, 2003
    #3
  4. Vijay Kumar R Zanvar

    Simon Biber Guest

    "Ronny Mandal" <> wrote:
    > When you operate on pointers, *p will give you access to what is
    > stored at the pointers address. Similar to ordinary variables:
    >
    > int p=5

    ;

    > printf ( "%d\n", p ); will yield 5


    It'll output the digit 5 and a newline character, yeah.

    > eq
    >
    > int *p = 5;


    This wrongly attempts to initialise a pointer type with an integer. It
    is a constraint violation, so the compiler must emit a diagnostic
    message. Perhaps you actually meant:
    int i = 5;
    int *p = &i;
    Now i has the value 5, and p has the value of the address of i.

    > printf ( "%d\n", *p ); will yield 5 also


    True, given my correction.

    > printf ( "%d\n", p ); will yield the address in memory where p is stored.


    This is undefined behaviour, as the %d conversion requires an int as its
    argument. The correct way to output a representation of the value of a
    pointer is:
    printf("%p\n", (void *)p);
    This converts the value of type 'pointer to int' into a value of type
    'pointer to void' as required by the %p conversion specifier.

    > Assuming that EXIT_SUCCESS is 0 (simply put in a 'define EXIT_SUCCESS 0')


    No! EXIT_SUCCESS is a macro defined in <stdlib.h>, which the OP Vijay
    correctly included. It has the same meaning as returning 0, but need
    not actually have the value 0. You are not allowed to define this
    macro yourself, that would be undefined behaviour.

    --
    Simon.
     
    Simon Biber, Dec 24, 2003
    #4
  5. Vijay Kumar R Zanvar

    nrk Guest

    Vijay Kumar R Zanvar wrote:

    > Hi,
    >
    > I invite reviews for the following code:
    >
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int
    > main ( void )
    > {
    > char *p;
    >
    > p = (char*) &p;
    > strcpy ( p, "Hi" );
    > printf ( "%s\n", p );
    > return EXIT_SUCCESS;
    > }
    >
    >
    > Thanks.
    >


    Crap.

    -nrk.
     
    nrk, Dec 24, 2003
    #5
  6. Vijay Kumar R Zanvar

    striker Guest

    On Wed, 24 Dec 2003 11:45:50 +0530, Vijay Kumar R Zanvar wrote:

    > Hi,

    hey
    >
    > I invite reviews for the following code:
    >
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int
    > main ( void )
    > {

    3 lines for a function definition? Well, guess it's ok...
    > char *p;
    >
    > p = (char*) &p;

    casting a (char **) to a (char *). Not very healthy.
    > strcpy ( p, "Hi" );

    now copying literal string "Hi" to *p. Fsck, Segfault!
    > printf ( "%s\n", p );

    If your O/S managed not to segfault then you'll see lots of crap in your
    terminal.
    > return EXIT_SUCCESS;

    Yeah, no errors at all.
    > }
    > }
    > }

    Those last braces are lost in the source.
    > Thanks.

    You're welcome
     
    striker, Dec 28, 2003
    #6
  7. On Sun, 28 Dec 2003 04:06:32 +0000, striker <>
    wrote:

    >On Wed, 24 Dec 2003 11:45:50 +0530, Vijay Kumar R Zanvar wrote:
    >
    >> Hi,

    >hey
    >>
    >> I invite reviews for the following code:
    >>
    >> #include <stdio.h>
    >> #include <string.h>
    >> #include <stdlib.h>
    >>
    >> int
    >> main ( void )
    >> {

    >3 lines for a function definition? Well, guess it's ok...
    >> char *p;
    >>
    >> p = (char*) &p;

    >casting a (char **) to a (char *). Not very healthy.


    Since char* and void* are required to have the same representation,
    why do you think this is a problem?

    >> strcpy ( p, "Hi" );

    >now copying literal string "Hi" to *p. Fsck, Segfault!


    Unless p happens to occupy less than three bytes (possibly true on
    some 16 bit systems), why do you think overlaying the bytes of p
    causes a segfault. By the way, lots of systems don't have segments
    and therefore cannot have segfaults.

    >> printf ( "%s\n", p );

    >If your O/S managed not to segfault then you'll see lots of crap in your
    >terminal.


    This one is more likely to cause a memory access failure than anything
    previous.

    >> return EXIT_SUCCESS;

    >Yeah, no errors at all.
    >> }
    >> }
    >> }

    >Those last braces are lost in the source.
    >> Thanks.

    >You're welcome




    <<Remove the del for email>>
     
    Barry Schwarz, Dec 28, 2003
    #7
  8. Vijay Kumar R Zanvar

    CBFalconer Guest

    Barry Schwarz wrote:
    > striker <> wrote:
    > > Vijay Kumar R Zanvar wrote:
    > >

    .... snip ...
    > >>
    > >> char *p;
    > >>
    > >> p = (char*) &p;

    > >
    > >casting a (char **) to a (char *). Not very healthy.

    >
    > Since char* and void* are required to have the same
    > representation, why do you think this is a problem?


    I see no void*. Why do you think a pointer to a pointer to a char
    necessarily has any similarity?

    --
    Chuck F () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
     
    CBFalconer, Dec 28, 2003
    #8
  9. On Sun, 28 Dec 2003, CBFalconer wrote:
    >
    > Barry Schwarz wrote:
    > > striker <> wrote:
    > > > Vijay Kumar R Zanvar wrote:
    > > >>
    > > >> char *p;
    > > >>
    > > >> p = (char*) &p;
    > > >
    > > >casting a (char **) to a (char *). Not very healthy.

    > >
    > > Since char* and void* are required to have the same
    > > representation, why do you think this is a problem?

    >
    > I see no void*. Why do you think a pointer to a pointer to a char
    > necessarily has any similarity?


    I think Barry was trying to point out that the assignment,
    while "not very healthy," was in fact perfectly *legal* C code,
    via the similarity between

    void *foo = (void *) &p; /* obviously correct */
    and
    char *bar = (char *) &p; /* also correct */

    A (char *), AFAIK, is guaranteed to be able to point anywhere a
    (void *) can -- because a 'char' is the smallest addressable
    unit of memory in C.
    Now, I don't wish to beat Barry with a dead horse, but I have
    pointed out ad nauseam that just because (void *) must have the
    same representation as "a pointer to a character type," doesn't
    mean it must have the same representation as a pointer to 'char'
    *in particular*! So his statement, while well-intentioned, was
    a little off-target [unless that passage from N869 has been
    clarified when I wasn't paying attention].

    -Arthur
     
    Arthur J. O'Dwyer, Dec 29, 2003
    #9
  10. On Sun, 28 Dec 2003 22:41:13 GMT, CBFalconer <>
    wrote:

    >Barry Schwarz wrote:
    >> striker <> wrote:
    >> > Vijay Kumar R Zanvar wrote:
    >> >

    >... snip ...
    >> >>
    >> >> char *p;
    >> >>
    >> >> p = (char*) &p;
    >> >
    >> >casting a (char **) to a (char *). Not very healthy.

    >>
    >> Since char* and void* are required to have the same
    >> representation, why do you think this is a problem?

    >
    >I see no void*. Why do you think a pointer to a pointer to a char
    >necessarily has any similarity?


    &p has type pointer to pointer to char. Let's call this pointer to T.
    Any pointer can be converted (explicitly or implicitly) to type void*
    without problem. char* is required to have the same representation as
    void *. Therefore my question: Why did striker believe that
    explicitly casting a pointer to T to a char* would cause a problem?
    What kind of problem could it possibly cause?


    <<Remove the del for email>>
     
    Barry Schwarz, Dec 29, 2003
    #10
  11. "Ronny Mandal" <> wrote in message
    news:bsbd4d$c88$...
    > "Vijay Kumar R Zanvar" <> wrote in message
    > news:bsbapi$bfn8a$-berlin.de...

    <snip>
    > > char *p;
    > >
    > > p = (char*) &p;

    > Why cast the pointers address to the pointer?


    What about, "because it wouldn't compile otherwise?" (Hint: think types!)

    Simon Biber has already corrected your other errors and other posters
    corrected the OP, so I won't bother.

    Peter
     
    Peter Pichler, Dec 29, 2003
    #11
  12. On 28 Dec 2003 19:45:41 GMT, in comp.lang.c , Barry Schwarz
    <> wrote:

    >On Sun, 28 Dec 2003 04:06:32 +0000, striker <>
    >wrote:
    >
    >>On Wed, 24 Dec 2003 11:45:50 +0530, Vijay Kumar R Zanvar wrote:
    >>> strcpy ( p, "Hi" );

    >>now copying literal string "Hi" to *p. Fsck, Segfault!

    >
    >Unless p happens to occupy less than three bytes (possibly true on
    >some 16 bit systems), why do you think overlaying the bytes of p
    >causes a segfault.


    The size of p is not really relevant. It is uninitialised, copying
    anything into wherever it points is UB, and might well segfault ....

    >By the way, lots of systems don't have segments
    >and therefore cannot have segfaults.


    ..... even on a machine which doesn't have segfaults. Its UB. It can do
    anything it jolly well pleases.

    More pragmatically, its quite possible that the particular arch used
    by the OP points all uninitialised pointers at readonly memory, or at
    an invalid address.

    --
    Mark McIntyre
    CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
    CLC readme: <http://www.angelfire.com/ms3/bchambless0/welcome_to_clc.html>


    ----== Posted via Newsfeed.Com - Unlimited-Uncensored-Secure Usenet News==----
    http://www.newsfeed.com The #1 Newsgroup Service in the World! >100,000 Newsgroups
    ---= 19 East/West-Coast Specialized Servers - Total Privacy via Encryption =---
     
    Mark McIntyre, Jan 2, 2004
    #12
  13. Mark McIntyre <> wrote in message news:<>...
    > On 28 Dec 2003 19:45:41 GMT, in comp.lang.c , Barry Schwarz
    > <> wrote:
    >
    > >On Sun, 28 Dec 2003 04:06:32 +0000, striker <>
    > >wrote:
    > >
    > >>On Wed, 24 Dec 2003 11:45:50 +0530, Vijay Kumar R Zanvar wrote:
    > >>> strcpy ( p, "Hi" );
    > >>now copying literal string "Hi" to *p. Fsck, Segfault!

    > >
    > >Unless p happens to occupy less than three bytes (possibly true on
    > >some 16 bit systems), why do you think overlaying the bytes of p
    > >causes a segfault.

    >
    > The size of p is not really relevant. It is uninitialised, copying
    > anything into wherever it points is UB, and might well segfault ....


    If you go back to my message which you responded to and go up 9 lines
    from the "Unless" line, you will see that the OP initialized p with
    the statement
    p = (char*)&p;
    so that p points to an area of memory exactly sizeof p bytes long. As
    long as p occupies at least three bytes, there is no undefined
    behavior associated with the call to strcpy.

    >
    > >By the way, lots of systems don't have segments
    > >and therefore cannot have segfaults.

    >
    > .... even on a machine which doesn't have segfaults. Its UB. It can do
    > anything it jolly well pleases.


    It is not UB unless sizeof p < 3.

    >
    > More pragmatically, its quite possible that the particular arch used
    > by the OP points all uninitialised pointers at readonly memory, or at
    > an invalid address.


    Not relevant since the pointer is initialized.
     
    Barry Schwarz, Jan 3, 2004
    #13
    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. Volodymyr Sadovyy

    Code write \ code review productivity

    Volodymyr Sadovyy, Apr 23, 2004, in forum: Java
    Replies:
    8
    Views:
    782
    Roedy Green
    Apr 25, 2004
  2. Otto Wyss
    Replies:
    5
    Views:
    449
    Robert Vazan
    Sep 7, 2003
  3. andrew blah
    Replies:
    6
    Views:
    370
    andrew blah
    Oct 17, 2004
  4. Josiah Carlson
    Replies:
    1
    Views:
    361
    Andrew Clover
    Oct 13, 2004
  5. www
    Replies:
    51
    Views:
    1,506
Loading...

Share This Page