On 03/03/2014 22:12, Mark Cave-Ayland wrote:
On 18/02/14 01:08, Olivier Danet wrote:
Hello ! I have tried to recreate the 3 known variants of next-property:
Standard : a) true "string" b) false c) false Sun Ultra10 : a) -1 "string" b) nothing c) nothing PowerPC iBook : a) -1 "string" b) -1 0 0 c) 0
Here is a tentative patch :
Index: forth/admin/devices.fs
--- forth/admin/devices.fs (révision 1269) +++ forth/admin/devices.fs (copie de travail) @@ -451,7 +451,7 @@ ?active-package dup >r if 0 0 begin
- r@ next-property
- r@ next-property-std
while cr 2dup dup -rot type begin ." " 1+ dup d# 26 >= until drop Index: forth/device/property.fs =================================================================== --- forth/device/property.fs (révision 1269) +++ forth/device/property.fs (copie de travail) @@ -63,14 +63,14 @@ ;
\ From package (5.3.4.1) -: next-property +: next-property-std ( previous-str previous-len phandle -- false | name-str name-len true )
r
2dup 0= swap 0= or if 2drop r> >dn.properties @ else r> find-property dup if @ then
- ?dup if >prop.next @ then
- dup if >prop.next @ then
then
?dup if @@ -81,7 +81,57 @@ then ;
+: next-property-ppc +( previous-str previous-len phandle -- false | name-str name-len true | 0 0 true )
r
2dup 0= swap 0= or if
2drop r> >dn.properties @ true
else
r> find-property dup if @ then
dup if >prop.next @ true then
then
if
dup if
prop.name @ dup cstrlen
( phandle name-str name-len )
else
0
( 0 0 )
then
true
else
false
then
+;
+: next-property-std-bis
- next-property-ppc
- dup
- if over 0=
- if
- 2drop
- then
- then
+;
+: next-property-s64
- next-property-std
- ?dup drop
+;
+: next-property
- [IFDEF] CONFIG_PPC
- next-property-ppc
- [ELSE]
- [IFDEF] CONFIG_SPARC64
- next-property-s64
- [ELSE]
- next-property-std
- [THEN]
- [THEN]
+;
\ \ 5.3.5.4 Property value access \ Index: forth/system/ciface.fs =================================================================== --- forth/system/ciface.fs (révision 1269) +++ forth/system/ciface.fs (copie de travail) @@ -121,7 +121,7 @@
( buf prev prev_len )
- r> next-property if
- r> next-property-std if
( buf name name_len ) dup 1+ -rot ci-strcpy drop 1 else ===================================================================
there are : next-property-std = standard, used by Sparc32 "romvec" C code and by forth code, like ".properties" next-property-s64 = Sparc64 version, built from next-property-std next-property-ppc = PowerPC version next-property-std-bis = Alternate standard version, built from the PPC one (e.g. when compiling for PPC target)
Tests :
showstack cd screen
\ Sparc32 " " ?active-package next-property-std cr . type cr " name" ?active-package next-property-std cr . type cr " interrupts" ?active-package next-property-std cr . cr " ttttt" ?active-package next-property-std cr . cr
\ PowerPC " " ?active-package next-property-ppc cr . type cr " name" ?active-package next-property-ppc cr . type cr " interrupts" ?active-package next-property-ppc cr . . . cr " ttttt" ?active-package next-property-ppc cr . cr
\ Sparc64 " " ?active-package next-property-s64 cr . type cr " name" ?active-package next-property-s64 cr . type cr " interrupts" ?active-package next-property-s64 cr " ttttt" ?active-package next-property-s64 cr
" " ?active-package next-property-std-bis cr . type cr " name" ?active-package next-property-std-bis cr . type cr " interrupts" ?active-package next-property-std-bis cr . cr " ttttt" ?active-package next-property-std-bis cr . cr
Currently all variants are available simultaneously. Additional [IFDEF] can be used to hide some variants according to compilation options, generate the -std from the -ppc,..
One more oddity: NetBSD is not the only one to do strange things with properties. In OpenBSD, one can find : arch/sparc/dev/sbus.c:int sbus_testdma(sc, ca) ... getpropint(0, "slave-only", 0) ... arch/sparc/dev/sbus.c:int sbus_print(args, sbus) ... getpropint(0, sl, 0) ... Here, 0 should select the 'root' node. It really looks like a bug in OpenBSD. It does not prevent the OS from running though.
Olivier
Hi Olivier,
Thanks for doing this patch, and apologies for the delay in getting back to you as it has given me quite a few things to think about!
My current thoughts at the moment are that the reason these particular differences in next-property have only just been discovered is because it is extremely unlikely that anyone would call it directly. So as we discovered from Zoltan, PPC/SPARC64 would call next-property via the client interface, and we know that SPARC32 would call next-property via the romvec block.
I guess the only way these bugs could be encountered would be if someone evaluated a Forth string containing next-property directly, and why would they bother if OpenBIOS already provides a method to do this via CIF/romvec?
Therefore what I would suggest is that we hold on this patch for the moment and instead could you provide an alternative that fixes the return from next-property in obp_nextprop instead? That way we have the results of your research preserved in the archives in case we need to rethink this later, plus the patch itself is much simpler making it much less likely to affect other platforms.
ATB,
Mark.
Just push the one-liner Forth patch I submitted two months ago...
Regards. Olivier