On 02/01/14 01:43, Olivier Danet wrote:
Hi Olivier,
Happy New Year!
I may have found a bug in OpenBIOS.
An OS uses the "nextprop" iterator to scan device properties, and eventually asks about nonexistant properties. "const char *obp_nextprop(int node, const char *name)" (arch/sparc32/romvec.c) calls the "next-property" forth code, then pops the result. According to the standard, next-property should return 0 if the property selected is the last _OR_ if the property does not exist. The current code in forth/admin/devices.fs returns nothing when the property does not exit.
For example (SPARC 32) :
cd screen " name" ?active-package next-property drop cr type --> returns "device_type"
" interrupts" ?active-package next-property --> returns 0, this is the last property.
" bozo" ?active-package next-property --> returns nothing. Which, imho, is wrong.
Here is a patch, I am not quite sure about it (debuting Forth) but it helps the target OS :
Index: forth/device/property.fs
--- forth/device/property.fs (révision 1246) +++ forth/device/property.fs (copie de travail) @@ -70,7 +70,7 @@ 2drop r> >dn.properties @ else r> find-property dup if @ then
- ?dup if >prop.next @ then
- dup if >prop.next @ then
then
?dup if
I've had a quick look at the patch and it looks good to me, although I'd probably like to test it on a few more OSs first before commit ;)
The standard asks for a pointer to a zero lenght string instead of zero, the C code obp_nextprop() expects a zero. I don't know if it makes a difference.
Here the standard looks a bit confused with respect to next-property as it's clearly talking about NULL-terminated (C strings) as opposed to Forth strings ( addr len ) on the stack. Having said that, I'm fairly sure that string properties are stored NULL-terminated anyway although it's probably worth checking the contents of str in obp_nextprop() with gdb just to make sure.
Olivier (Btw, the OS is NetBSD :-)
Oh that's interesting. As part of my test suite, I regularly boot SPARC32 NetBSD into graphics mode so I would expect this to already work. Or is the key thing here that because of the screen node you are working on hardware framebuffer support? :)
ATB,
Mark.