zsh-workers
 help / color / mirror / code / Atom feed
* Fishier code in handlefeatures
@ 2015-01-06  0:20 Mikael Magnusson
  2015-01-06  0:24 ` ZyX
  0 siblings, 1 reply; 5+ messages in thread
From: Mikael Magnusson @ 2015-01-06  0:20 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh workers

mod_export int
handlefeatures(Module m, Features f, int **enables)
{
    if (!enables || *enables)
      return setfeatureenables(m, f, *enables);
    *enables = getfeatureenables(m, f);
    return 0;
}

so if enables is NULL, we immediately do *enables? I'm not sure what's
intended here but obviously it somehow works.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fishier code in handlefeatures
  2015-01-06  0:20 Fishier code in handlefeatures Mikael Magnusson
@ 2015-01-06  0:24 ` ZyX
  2015-01-06  0:32   ` Mikael Magnusson
  0 siblings, 1 reply; 5+ messages in thread
From: ZyX @ 2015-01-06  0:24 UTC (permalink / raw)
  To: Mikael Magnusson, Peter Stephenson; +Cc: zsh workers

06.01.2015, 03:21, "Mikael Magnusson" <mikachu@gmail.com>:
> mod_export int
> handlefeatures(Module m, Features f, int **enables)
> {
>     if (!enables || *enables)
>       return setfeatureenables(m, f, *enables);
>     *enables = getfeatureenables(m, f);
>     return 0;
> }
>
> so if enables is NULL, we immediately do *enables? I'm not sure what's
> intended here but obviously it somehow works.

NULL is false, so if enables is NULL !enables is true and *enables is not evaluated due to short-circuiting. Nothing wrong here.

>
> --
> Mikael Magnusson


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fishier code in handlefeatures
  2015-01-06  0:24 ` ZyX
@ 2015-01-06  0:32   ` Mikael Magnusson
  2015-01-06  0:36     ` ZyX
  0 siblings, 1 reply; 5+ messages in thread
From: Mikael Magnusson @ 2015-01-06  0:32 UTC (permalink / raw)
  To: ZyX; +Cc: Peter Stephenson, zsh workers

On Tue, Jan 6, 2015 at 1:24 AM, ZyX <kp-pav@yandex.ru> wrote:
> 06.01.2015, 03:21, "Mikael Magnusson" <mikachu@gmail.com>:
>> mod_export int
>> handlefeatures(Module m, Features f, int **enables)
>> {
>>     if (!enables || *enables)
>>       return setfeatureenables(m, f, *enables);
>>     *enables = getfeatureenables(m, f);
>>     return 0;
>> }
>>
>> so if enables is NULL, we immediately do *enables? I'm not sure what's
>> intended here but obviously it somehow works.
>
> NULL is false, so if enables is NULL !enables is true and *enables is not evaluated due to short-circuiting. Nothing wrong here.

Read the next line too.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fishier code in handlefeatures
  2015-01-06  0:32   ` Mikael Magnusson
@ 2015-01-06  0:36     ` ZyX
  2015-01-06  7:45       ` Bart Schaefer
  0 siblings, 1 reply; 5+ messages in thread
From: ZyX @ 2015-01-06  0:36 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Peter Stephenson, zsh workers

06.01.2015, 03:32, "Mikael Magnusson" <mikachu@gmail.com>:
> On Tue, Jan 6, 2015 at 1:24 AM, ZyX <kp-pav@yandex.ru> wrote:
>>  06.01.2015, 03:21, "Mikael Magnusson" <mikachu@gmail.com>:
>>>  mod_export int
>>>  handlefeatures(Module m, Features f, int **enables)
>>>  {
>>>      if (!enables || *enables)
>>>        return setfeatureenables(m, f, *enables);
>>>      *enables = getfeatureenables(m, f);
>>>      return 0;
>>>  }
>>>
>>>  so if enables is NULL, we immediately do *enables? I'm not sure what's
>>>  intended here but obviously it somehow works.
>>  NULL is false, so if enables is NULL !enables is true and *enables is not evaluated due to short-circuiting. Nothing wrong here.
>
> Read the next line too.

Ah. You mean dereference in function arguments, good catch.

Guess this code means that `enables` is never NULL and first part of the condition should be dropped in order not to confuse people.

>
> --
> Mikael Magnusson


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fishier code in handlefeatures
  2015-01-06  0:36     ` ZyX
@ 2015-01-06  7:45       ` Bart Schaefer
  0 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2015-01-06  7:45 UTC (permalink / raw)
  To: zsh workers

On Jan 6,  1:20am, Mikael Magnusson wrote:
}
}     if (!enables || *enables)
}       return setfeatureenables(m, f, *enables);
} 
} so if enables is NULL, we immediately do *enables? I'm not sure what's
} intended here but obviously it somehow works.

On Jan 6,  3:36am, ZyX wrote:
} 
} Guess this code means that `enables` is never NULL and first part of
} the condition should be dropped in order not to confuse people.

setfeatureenables() accepts a NULL third argument and is called that way
in a number of places; so I suspect that even though none of the current
module implementations calls handlefeatures() with a NULL third argument,
it was probably intended to permit that case.

None of this is described in zsh-development-guide, though.


diff --git a/Src/module.c b/Src/module.c
index 9e8b3cc..bcc0ba8 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -3367,7 +3367,7 @@ mod_export int
 handlefeatures(Module m, Features f, int **enables)
 {
     if (!enables || *enables)
-	return setfeatureenables(m, f, *enables);
+	return setfeatureenables(m, f, enables ? *enables : NULL);
     *enables = getfeatureenables(m, f);
     return 0;
 }

-- 
Barton E. Schaefer


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-01-06  7:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-06  0:20 Fishier code in handlefeatures Mikael Magnusson
2015-01-06  0:24 ` ZyX
2015-01-06  0:32   ` Mikael Magnusson
2015-01-06  0:36     ` ZyX
2015-01-06  7:45       ` Bart Schaefer

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).