[OpenBIOS] [PATCH] libopenbios/bootinfo_load.c: Implement Mac OS 9.2 boot script support

[Segher Boessenkool]

On Sun, May 01, 2016 at 08:27:59PM -0400, Programmingkid wrote:
> > I think Segher's version is closer to what we are trying to achieve -
> > there is a lot of extra code added for something where the end result
> > isn't excessively context. Can you find out with the Forth debugger
> > exactly why Segher's patch is failing?

It's not my patch that is failing, there is just something else than
before failing in the boot script, in a worse way it seems.

> I decided to recreate his patch. Here it is below:
> 
> Index: forth/lib/vocabulary.fs
> ===================================================================
> --- forth/lib/vocabulary.fs	(revision 1395)
> +++ forth/lib/vocabulary.fs	(working copy)
> @@ -151,3 +151,46 @@
>    ;
>   
>  true to vocabularies?
> +
> +VARIABLE max-stack-size
> +100 max-stack-size !

You forgot to make it hex, making it a much smaller number.  Not that
that matters too much here, twenty or so entries is more than most
things need.  Anyway, your Forth code is worse than your C, time to
learn a bit :-)

   h# 100 CONSTANT max-stack-size

since it is a constant and not a variable.

> +: stash-push ( n -- ) 
> +    \ check for overflow before pushing value
> +    stash-stack-pointer @
> +    max-stack-size @ 1-
> +    >

Just use  >=  .

> +    if ." Warning: stack limit reached" cr EXIT then

That is not going to end well.  Warning and then ignoring an error
does not help much.  You also forgot to drop the value to push.
Just use  ABORT"  :

   stash-stack-pointer ( not a pointer at all, it's an array index)
   max-stack-size >= ABORT" Oh bah, we blew our stack"


> +    stash-stack-pointer @ 1 +     \ calculate new value for stack pointer
> +    stash-stack-pointer !   \ increment the stack pointer

   1 stash-stack-pointer +!

"1 +" is usually written "1+" as well, fwiw.  It is faster on some
systems, but more importantly, it is more idiomatic, easier to read.

> +: stash-pop ( -- n )
> +    stash-stack-pointer @ 1 -   \ calculate new value for the stack pointer
> +    dup ( new-stack-pointer new-stack-pointer )
> +    0 ( new-stack-pointer new-stack-pointer 0)
> +    < ( new-stack-pointer boolean )
> +    if ( new-stack-pointer )
> +        drop 0 ( 0 )
> +        ." Warning: stack underflow detected" cr 
> +    else 
> +        ( new-stack-pointer )
> +    then
> +    stash-stack-pointer !   \ store new value in stack pointer

So, it's fine to decrement it and store it, and only then check it
(also note how to check for negative):

   -1 stash-stack-pointer +!
   stash-stack-pointer @ 0<
      ABORT" whoops we drained the stack a bit too much"

Cheers,


Segher