instance: Fix set-my-args for empty arguments

It would put the pointer and len in the wrong order in the instance>args
buffer.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 slof/fs/instance.fs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 08.06.2016 11:35, Benjamin Herrenschmidt wrote:
> It would put the pointer and len in the wrong order in the instance>args
> buffer.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  slof/fs/instance.fs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs
> index 9e5c921..225a2bf 100644
> --- a/slof/fs/instance.fs
> +++ b/slof/fs/instance.fs
> @@ -134,7 +134,7 @@ CONSTANT <instancevariable>
>        2dup my-self instance>args 2!   \ | write into instance struct  ( old-addr len new-addr )
>        swap move                       \ | and copy the args           ( )
>     ELSE                               \ ELSE                          ( old-addr len )
> -      my-self instance>args 2!        \ | set new args to zero, too   ( )
> +      swap my-self instance>args 2!   \ | set new args to zero, too   ( )
>     THEN                               \ FI
>  ;

You're right, the old code stored the address and length in the wrong
order into instance>args and instance>args-len. However, the code likely
simply expected to write zeros to both fields instead, so maybe it is
better to do this instead:

	nip 0 my-self instance>args 2!

... that way you'd make sure that instance>args never contains a bogus
pointer if instance>args-len is 0.

 Thomas

On Wed, Jun 08, 2016 at 06:02:54PM +0200, Thomas Huth wrote:
> > diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs
> > index 9e5c921..225a2bf 100644
> > --- a/slof/fs/instance.fs
> > +++ b/slof/fs/instance.fs
> > @@ -134,7 +134,7 @@ CONSTANT <instancevariable>
> >        2dup my-self instance>args 2!   \ | write into instance struct  ( old-addr len new-addr )
> >        swap move                       \ | and copy the args           ( )
> >     ELSE                               \ ELSE                          ( old-addr len )
> > -      my-self instance>args 2!        \ | set new args to zero, too   ( )
> > +      swap my-self instance>args 2!   \ | set new args to zero, too   ( )
> >     THEN                               \ FI
> >  ;
> 
> You're right, the old code stored the address and length in the wrong
> order into instance>args and instance>args-len. However, the code likely
> simply expected to write zeros to both fields instead, so maybe it is
> better to do this instead:
> 
> 	nip 0 my-self instance>args 2!
> 
> ... that way you'd make sure that instance>args never contains a bogus
> pointer if instance>args-len is 0.

Why treat a length of zero special at all?  alloc-mem already knows how
to handle it (it returns 0 for the address).


Segher

On 08.06.2016 18:36, Segher Boessenkool wrote:
> On Wed, Jun 08, 2016 at 06:02:54PM +0200, Thomas Huth wrote:
>>> diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs
>>> index 9e5c921..225a2bf 100644
>>> --- a/slof/fs/instance.fs
>>> +++ b/slof/fs/instance.fs
>>> @@ -134,7 +134,7 @@ CONSTANT <instancevariable>
>>>        2dup my-self instance>args 2!   \ | write into instance struct  ( old-addr len new-addr )
>>>        swap move                       \ | and copy the args           ( )
>>>     ELSE                               \ ELSE                          ( old-addr len )
>>> -      my-self instance>args 2!        \ | set new args to zero, too   ( )
>>> +      swap my-self instance>args 2!   \ | set new args to zero, too   ( )
>>>     THEN                               \ FI
>>>  ;
>>
>> You're right, the old code stored the address and length in the wrong
>> order into instance>args and instance>args-len. However, the code likely
>> simply expected to write zeros to both fields instead, so maybe it is
>> better to do this instead:
>>
>> 	nip 0 my-self instance>args 2!
>>
>> ... that way you'd make sure that instance>args never contains a bogus
>> pointer if instance>args-len is 0.
> 
> Why treat a length of zero special at all?  alloc-mem already knows how
> to handle it (it returns 0 for the address).

You're right - it should work without the IF-statement, too!

 Thomas