[OpenBIOS] next-property

[Mark Cave-Ayland]

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.