more string input confusion

Discussion in 'C Programming' started by merrittr, May 16, 2007.

  1. merrittr

    merrittr Guest

    I have been having troubles getting "string" input pushed on to the
    stack in the assn1. The funny thing is
    I can push a literal on no problem so

    push("push i 0");
    push("push i 1");
    push("add");

    works fine however

    while(cVal != '\n') {
    cString[iCharCount++] = tolower(cVal);
    cVal=getchar();
    }
    cString[iCharCount++]='\0';
    iCharCount=0;
    push(cString);

    doesn't work is there a diference between the literal text string and
    the built string (char array null terminated?).
    I have avoided scanf and gets`after reading the c programming FAQ.



    here is the push code:
    void push(char *p)
    {
    int ptr=iSp;
    iItem[iSp++]=p;
    print(*iItem);
    }
     
    merrittr, May 16, 2007
    #1
    1. Advertising

  2. In article <>,
    merrittr <> wrote:
    >I have been having troubles getting "string" input pushed on to the
    >stack in the assn1. The funny thing is
    >I can push a literal on no problem so


    >push("push i 0");
    >push("push i 1");
    >push("add");


    >works fine however


    >while(cVal != '\n') {
    > cString[iCharCount++] = tolower(cVal);
    > cVal=getchar();
    >}
    >cString[iCharCount++]='\0';
    >iCharCount=0;
    >push(cString);


    >doesn't work is there a diference between the literal text string and
    >the built string (char array null terminated?).


    >here is the push code:
    >void push(char *p)
    >{
    >int ptr=iSp;
    >iItem[iSp++]=p;
    >print(*iItem);
    >}


    Your iItem[iSp++]=p code in push() copies the pointer, but probably
    on your next loop around you overwrite the same memory area cString
    again and push a copy of that same pointer. String literals, though,
    have persistant and unique addresses, so the recorded pointer for
    those is going to continue to point to the literal content, not overwritten
    by the next trip through the loop. If you were to push the -same-
    string literal again, and were to examine iItem[], you would find that
    you had two copies of the same literal pointer.

    What you probably need to do in your push() routine is
    find the strlen() of the string passed in, malloc() enough space
    to hold the string *and it's terminating null*, copy the
    string into that malloc'd memory, and record the pointer to the
    malloc'd memory; when you went to pop() the value, you would then need
    to free() the pointer after you had finished using it.
    --
    "No one has the right to destroy another person's belief by
    demanding empirical evidence." -- Ann Landers
     
    Walter Roberson, May 16, 2007
    #2
    1. Advertising

  3. merrittr said:

    > I have been having troubles getting "string" input pushed on to the
    > stack in the assn1. The funny thing is
    > I can push a literal on no problem so
    >
    > push("push i 0");
    > push("push i 1");
    > push("add");
    >
    > works fine however
    >
    > while(cVal != '\n') {
    > cString[iCharCount++] = tolower(cVal);
    > cVal=getchar();
    > }
    > cString[iCharCount++]='\0';
    > iCharCount=0;
    > push(cString);
    >
    > doesn't work


    You haven't shown enough code for us to tell precisely what is going
    wrong, but...

    > is there a diference between the literal text string and
    > the built string (char array null terminated?).


    ....string literals are indeed strings - i.e. null terminated arrays of
    char, just like what you call "built" strings (provided you build them
    properly, of course).

    > I have avoided scanf and gets`after reading the c programming FAQ.


    Very wise.

    > here is the push code:
    > void push(char *p)
    > {
    > int ptr=iSp;
    > iItem[iSp++]=p;
    > print(*iItem);
    > }


    Aha! Tell me... is the symptom that you get the same thing printed over
    and over again? I can see two aspects of your code that might both,
    separately or together, cause that problem.

    Note that iItem[iSp++]=p; does not create a fresh copy of the string. It
    merely points a pointer in the direction of the string. So if you give
    this function a bunch of string literals, it'll work fine, but every
    time you pass in your own "built" string as you call it, what you're
    really doing is telling your stack where that array is. The fact that
    you change the array's contents from time to time is of no concern to
    iItem[]. It will continue to point at that array, quite happily, and
    several times over if you tell it too.

    Note also that iItem[iSp++]=p suggests that iItem[iSp++] is a char *,
    and therefore *iItem is also a char *. This suggests that print() will
    only ever print the first item in the array!

    The solution to the first problem is to arrange some storage, and copy
    the data into it rather than just tweak pointers. The solution to the
    second is probably to populate iItem[iSp], then print(iItem[iSp]), and
    then increment iSp.

    Incidentally, one day you *will* get tired of type prefixes. Why not
    drop them now? That way, you won't have such a big clean-up job later
    on.

    --
    Richard Heathfield
    "Usenet is a strange place" - dmr 29/7/1999
    http://www.cpax.org.uk
    email: rjh at the above domain, - www.
     
    Richard Heathfield, May 16, 2007
    #3
  4. merrittr

    merrittr Guest

    Ah I see , thats great you 2 have been very helpfull here is the push
    function for the next person with the problem


    void push(char *p)
    {
    char *ptr;
    ptr = (char *)malloc( strlen(p) );
    strcpy(ptr, p);
    iItem[iSp++]=ptr;
    print(*iItem);
    }
     
    merrittr, May 16, 2007
    #4
  5. merrittr

    Ben Pfaff Guest

    merrittr <> writes:

    > void push(char *p)


    p could be made type "const char *".

    > {
    > char *ptr;
    > ptr = (char *)malloc( strlen(p) );


    You forgot to add 1 to the return value of strlen here.

    I don't recommend casting the return value of malloc():

    * The cast is not required in ANSI C.

    * Casting its return value can mask a failure to #include
    <stdlib.h>, which leads to undefined behavior.

    * If you cast to the wrong type by accident, odd failures can
    result.

    In unusual circumstances it may make sense to cast the return value of
    malloc(). P. J. Plauger, for example, has good reasons to want his
    code to compile as both C and C++, and C++ requires the cast, as he
    explained in article <9sFIb.9066$>.
    However, Plauger's case is rare indeed. Most programmers should write
    their code as either C or C++, not in the intersection of the two.

    > strcpy(ptr, p);
    > iItem[iSp++]=ptr;
    > print(*iItem);


    It seems odd for a push function to print the item at the bottom
    of the stack.

    > }
    >


    --
    Comp-sci PhD expected before end of 2007
    Seeking industrial or academic position *outside California* in 2008
     
    Ben Pfaff, May 16, 2007
    #5
  6. merrittr said:

    > Ah I see , thats great you 2 have been very helpfull here is the push
    > function for the next person with the problem
    >
    >


    Add:

    #include <stdlib.h> /* prototype for malloc */

    and

    #include <string.h> /* prototype for strcpy */

    > void push(char *p)
    > {
    > char *ptr;
    > ptr = (char *)malloc( strlen(p) );


    Lose the cast. Add 1 to strlen(p) to make room for the null terminator.
    And add this check:

    if(ptr != NULL)
    {

    > strcpy(ptr, p);
    > iItem[iSp++]=ptr;
    > print(*iItem);


    }

    > }


    Change the return type from void to int, and document which value you
    will return in the case of success, and which in the case of failure.
    One common convention is that 0 means success, and non-0 means
    something went wrong.

    --
    Richard Heathfield
    "Usenet is a strange place" - dmr 29/7/1999
    http://www.cpax.org.uk
    email: rjh at the above domain, - www.
     
    Richard Heathfield, May 16, 2007
    #6
  7. In article <>,
    merrittr <> wrote:
    >Ah I see , thats great you 2 have been very helpfull here is the push
    >function for the next person with the problem


    >void push(char *p)
    >{
    >char *ptr;
    >ptr = (char *)malloc( strlen(p) );
    >strcpy(ptr, p);
    >iItem[iSp++]=ptr;
    >print(*iItem);
    >}


    1) You don't need to cast the result of the malloc(), and doing so
    can sometimes hide errors

    2) strlen() does not include the terminating null character but
    strcpy() copies until (and including) null, so you are writing one
    more character than you have allocated for

    3) you do not check that the return value of the malloc is not NULL
    before you write to it.

    4) Do you really wish to be always printing the first iItem rather
    than the iItem you just pushed?

    5) what is print() anyhow? If that's a typo for printf() then be
    careful because your stored string might happen to include % format
    codes that printf() would try to interpret if the string is passed
    as the first parameter.
    --
    "law -- it's a commodity"
    -- Andrew Ryan (The Globe and Mail, 2005/11/26)
     
    Walter Roberson, May 16, 2007
    #7
  8. CBFalconer said:

    > Richard Heathfield wrote:
    >> merrittr said:
    >>

    > ... snip ...
    >>
    >>> here is the push code:
    >>> void push(char *p)
    >>> {
    >>> int ptr=iSp;
    >>> iItem[iSp++]=p;
    >>> print(*iItem);
    >>> }

    >>
    >> Aha! Tell me... is the symptom that you get the same thing printed
    >> over and over again? I can see two aspects of your code that might
    >> both, separately or together, cause that problem.

    >
    > Huh? iSp is undefined. As is iItem and print.


    It is not unreasonable to say so. Nor, however, is it unreasonable to
    assume that they are defined in unposted code.

    --
    Richard Heathfield
    "Usenet is a strange place" - dmr 29/7/1999
    http://www.cpax.org.uk
    email: rjh at the above domain, - www.
     
    Richard Heathfield, May 18, 2007
    #8
    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. harry
    Replies:
    16
    Views:
    613
    Roedy Green
    May 11, 2004
  2. Michael
    Replies:
    4
    Views:
    417
    Matt Hammond
    Jun 26, 2006
  3. lovecreatesbeauty
    Replies:
    17
    Views:
    657
    Keith Thompson
    Jun 16, 2006
  4. Jeff

    more auto_ptr confusion

    Jeff, May 17, 2006, in forum: C++
    Replies:
    23
    Views:
    871
    Noah Roberts
    May 19, 2006
  5. Robert Klemme

    With a Ruby Yell: more, more more!

    Robert Klemme, Sep 28, 2005, in forum: Ruby
    Replies:
    5
    Views:
    218
    Jeff Wood
    Sep 29, 2005
Loading...

Share This Page