more string input confusion

[merrittr]

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);
}

 

[Walter Roberson]

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.

 

[Richard Heathfield]

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.

 

[merrittr]

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);
}

 

[Ben Pfaff]

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 <[email protected]>.
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.

 

[Richard Heathfield]

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

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.

 

[Walter Roberson]

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.

 

[Richard Heathfield]

CBFalconer said:

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.