| Message ID | 1471840054-43361-1-git-send-email-aik@ozlabs.ru |
|---|---|
| State | Accepted |
| Headers | show |
On 22.08.2016 06:27, Alexey Kardashevskiy wrote: > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > It would put the pointer and len in the wrong order in the instance>args > buffer. As alloc-mem can handle zero length itself (and return NULL), > this also removes "if" to make the code simpler. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > [aik: removed "if" as Segher suggested] > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > slof/fs/instance.fs | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs > index 9e5c921..9159a5d 100644 > --- a/slof/fs/instance.fs > +++ b/slof/fs/instance.fs > @@ -129,13 +129,9 @@ CONSTANT <instancevariable> > > \ copy args from original instance to new created > : set-my-args ( old-addr len -- ) > - dup IF \ IF len > 0 ( old-addr len ) > - dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) > - 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 ( ) > - THEN \ FI > + dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) > + 2dup my-self instance>args 2! \ | write into instance struct ( old-addr len new-addr ) > + swap move \ | and copy the args ( ) I think you could now remove the pipe --^ characters in the comment now that the IF-statement is gone. Apart from that cosmetic nit, the patch looks fine to me. Reviewed-by: Thomas Huth <thuth@redhat.com>
On 23/08/16 08:29, Thomas Huth wrote: > On 22.08.2016 06:27, Alexey Kardashevskiy wrote: >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> >> It would put the pointer and len in the wrong order in the instance>args >> buffer. As alloc-mem can handle zero length itself (and return NULL), >> this also removes "if" to make the code simpler. >> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> [aik: removed "if" as Segher suggested] >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> slof/fs/instance.fs | 10 +++------- >> 1 file changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs >> index 9e5c921..9159a5d 100644 >> --- a/slof/fs/instance.fs >> +++ b/slof/fs/instance.fs >> @@ -129,13 +129,9 @@ CONSTANT <instancevariable> >> >> \ copy args from original instance to new created >> : set-my-args ( old-addr len -- ) >> - dup IF \ IF len > 0 ( old-addr len ) >> - dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) >> - 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 ( ) >> - THEN \ FI >> + dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) >> + 2dup my-self instance>args 2! \ | write into instance struct ( old-addr len new-addr ) >> + swap move \ | and copy the args ( ) > > I think you could now remove the pipe --^ characters in the comment now > that the IF-statement is gone. Apart from that cosmetic nit, the patch > looks fine to me. Yup, pushed without pipes, thanks for the review. > Reviewed-by: Thomas Huth <thuth@redhat.com> >
diff --git a/slof/fs/instance.fs b/slof/fs/instance.fs index 9e5c921..9159a5d 100644 --- a/slof/fs/instance.fs +++ b/slof/fs/instance.fs @@ -129,13 +129,9 @@ CONSTANT <instancevariable> \ copy args from original instance to new created : set-my-args ( old-addr len -- ) - dup IF \ IF len > 0 ( old-addr len ) - dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) - 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 ( ) - THEN \ FI + dup alloc-mem \ | allocate space for new args ( old-addr len new-addr ) + 2dup my-self instance>args 2! \ | write into instance struct ( old-addr len new-addr ) + swap move \ | and copy the args ( ) ; \ Current node has already been set, when this is called.