Message ID | 1465378500.3263.62.camel@kernel.crashing.org |
---|---|
State | Superseded |
Headers | show |
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
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 ;
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(-)