Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > more string input confusion

Reply
Thread Tools

more string input confusion

 
 
merrittr
Guest
Posts: n/a
 
      05-16-2007
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);
}

 
Reply With Quote
 
 
 
 
Walter Roberson
Guest
Posts: n/a
 
      05-16-2007
In article <(E-Mail Removed). com>,
merrittr <(E-Mail Removed)> 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
 
Reply With Quote
 
 
 
 
Richard Heathfield
Guest
Posts: n/a
 
      05-16-2007
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.
 
Reply With Quote
 
merrittr
Guest
Posts: n/a
 
      05-16-2007
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);
}

 
Reply With Quote
 
Ben Pfaff
Guest
Posts: n/a
 
      05-16-2007
merrittr <(E-Mail Removed)> 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$(E-Mail Removed)>.
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
 
Reply With Quote
 
Richard Heathfield
Guest
Posts: n/a
 
      05-16-2007
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.
 
Reply With Quote
 
Walter Roberson
Guest
Posts: n/a
 
      05-16-2007
In article <(E-Mail Removed) .com>,
merrittr <(E-Mail Removed)> 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)
 
Reply With Quote
 
Richard Heathfield
Guest
Posts: n/a
 
      05-17-2007
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.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Kamaelia 0.4.0 RELEASED - Faster! More Tools! More Examples! More Docs! ;-) Michael Python 4 06-26-2006 08:00 AM
more auto_ptr confusion Jeff C++ 23 05-19-2006 04:46 AM
More confusion on Canon cartridge code #s MB_ Digital Photography 0 01-07-2006 07:08 PM
With a Ruby Yell: more, more more! Robert Klemme Ruby 5 09-29-2005 06:37 AM
Passing by reference, more confusion... harry Java 16 05-11-2004 09:11 PM



Advertisments