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

[Andreas Färber]

Am 16.08.2011 um 04:04 schrieb William Hahne:

> On 8/15/11, Andreas Färber <andreas.faerber at web.de> 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.

I have access to both Apple and IBM ppc64 machines and could check if  
you provide paste'able Forth code. :)

> This may not be applicable to sparc at all but again
> someone with access to the hardware would have to confirm.

In its present form, the patch seems in no way limited to ppc - you  
might want to look at the [IFDEF word or so if that's your intent.

Also note, your division by 4 leaves up to three trailing bytes un- 
filled for unaligned lengths. If intentional, you should add a comment  
saying so.

Further, it was suggested we use QEMU coding style for new  
contributions - the above function uses an inconsistent indentation.

Andreas