[asterisk-dev] dubious code in channel.c

[Luigi Rizzo]

recent changes to channel.c caught my attention.
There are a few dubious segments of code:

+ In ast_write(), in most cases when we don't have a handler
  function for a frame type, the frame is just ignored
  and ast_write() returns 0;
  but in the case of AST_FRAME_VOICE and AST_FRAME_CONTROL,
  the function returns -1 resulting in SOFTHANGHUP being set.
  I don't know if this is intentional or not;

+ in __ast_read(), the section within ZAPTEL_OPTIMIZATIONS
  that calls
	ioctl(chan->timingfd, ZT_TIMERCONFIG, &blah);
  does that by releasing and reacquiring the channel lock
  just before the call. Is this intentional to give
  some other code a chance to run, or we can just move
  the previous unlock in the if() path and avoid the
  unlock/lock sequence ?

+ what is exactly the intended behaviour of the code with
  and without #ifndef MONITOR_CONSTANT_DELAY ?
  The code is probably implementing some kind of
  heuristic, but it is highly obfuscated, uncommented, and
  repeated twice in the read and write paths.
  Without a description of the intended behaviour,
  how can we tell that it is correct ?

comments anyone ? Kevin ?

cheers
luigi