Am 16.08.2011 um 04:04 schrieb William Hahne:
On 8/15/11, Andreas Färber andreas.faerber@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