Re: [OpenBIOS] [PATCH 10/10] Adding filll Forth primitive

[Andreas Färber]

Am 09.08.2011 um 23:55 schrieb William Hahne:
> This is a forth primitive that is required by BootX. It just fills  
> some specified memory address and length with longs.
> Index: kernel/forth.c
>
> --- kernel/forth.c	(revision 1041)
> +++ kernel/forth.c	(working copy)
> @@ -1610,6 +1616,20 @@
>    memset(src, value, count);
>  }
>
> +/*
> filll       ( addr len byte -- )
> */
>
> +static void filll(void)
> +{
> ucell value = POP();
> ucell count = POP();
> ucell *dest = (ucell *)cell2pointer(POP());
> int i;
> for (i = 0; i <= count / 4; i++) {
>    dest[i] = value;	
> }
>
> +}

This word puzzles me: The extra l in filll seems to be for "long"  
according to the patch description. But there's a hardcoded arithmetic  
with 4 and the parameter is documented as byte yet is being written  
ucell-wide. That strikes me as inconsistent.

Either 4 needs to be replaced with sizeof(ucell) to fit sparc64, or  
the division by 4 dropped and uint8_t* written len times.

Andreas

[PATCH 10/10] Adding filll Forth primitive

[William Hahne]

On 8/15/11, Andreas Färber  wrote:
> Am 09.08.2011 um 23:55 schrieb William Hahne:
> > This is a forth primitive that is required by BootX. It just fills
> > some specified memory address and length with longs.
> > Index: kernel/forth.c
> >
> > --- kernel/forth.c	(revision 1041)
> > +++ kernel/forth.c	(working copy)
> > @@ -1610,6 +1616,20 @@
> >    memset(src, value, count);
> >  }
> >
> > +/*
> > filll       ( addr len byte -- )
> > */
> >
> > +static void filll(void)
> > +{
> > ucell value = POP();
> > ucell count = POP();
> > ucell *dest = (ucell *)cell2pointer(POP());
> > int i;
> > for (i = 0; i <= count / 4; i++) {
> >    dest[i] = value;	
> > }
> >
> > +}
>
> This word puzzles me: The extra l in filll seems to be for "long"
> according to the patch description. But there's a hardcoded arithmetic
> with 4 and the parameter is documented as byte yet is being written
> ucell-wide. That strikes me as inconsistent.

Honestly, my problem is I was unable to find documentation on filll.
This suggests that it may be an non-standard addition by Apple. You
are right about the inconsistency. I will make a revised version of
the patch to address this.
> Either 4 needs to be replaced with sizeof(ucell) to fit sparc64, or
> the division by 4 dropped and uint8_t* written len times.

It seems likely that it is supposed to fill with 32bit integers, but
this would have to be checked on a 64bit platform which I do not have
access to. This may not be applicable to sparc at all but again
someone with access to the hardware would have to confirm.

William Hahne
> Andreas