[asterisk-dev] ast_flags now uint64_t ? should it be reverted ?

[Steve Murphy]

On Wed, 2007-07-18 at 07:13 -0700, Luigi Rizzo wrote:
> Hi,
> the following change modified ast_flags to uint64_t
> 
> 	http://svn.digium.com/view/asterisk?view=rev&revision=75400
> 
> causing a rather massive change, both in-tree and to third party
> code, all for the benefit of a single application, app_dial.c
> 
> I don't think this change is a good idea, partly because of the overall
> increase in memory and processing time (especially on low-end boxes),
> but mainly because it might open the way to hard-to-detect bugs
> because of truncation in assignments.
> 

Luigi--

You have a definite point; but I am not so concerned about the increase
in memory or time, as both would be negligible, even with embedded
situations. Most shifts and compares are unit cycle operations, so an
extra cycle should not seriously impact most algorithms. Flags are not
that frequently used in the grand scheme of things.

The compiler's been pretty good about telling us about truncation
issues, so I'm not overly concerned there, also. 


> I'd suggest to have this change reverted, and if that one application
> (app_dial.c) needs a larger set of flags, consider storing some of
> them in a different variable.
> 
> As an example, in app_dial.c, there is no reason to have
> DIAL_STILLGOING and DIAL_NOFORWARDHTML stored together
> with the various OPT_* flags. This can be fixed trivially.
> 

Pulling these out of the OPT list would buy us a little time, but Dial
is a pretty critical app in Asterisk. As we go along, we uncover new
needs and demands, and because Dial covers a fairly significant set of
operations that are critical to almost every use of asterisk, it's built
up a fairly large set of options.

Because Dial covers call setup (ring a set of phones, pick a winner), as
well as bridging, it'd be difficult to pull it apart into chunks. It
would complicate
dialplans and not necessarily make life better.

Look at it a different way: The way options are set up, and used at the
current moment, a total of 52 case-sensitive-single-letter options (62
if you allow using the numbers 0-9 also) are available to every
application. But flags can only deal with 31-32 each. So, using flags to
hold app options is fundamentally flawed.


But I agree with you-- the principle "minimal change" seems better; what
if I
created an analog to ast_flags, like ast_flags64, and a few APP_ARG
alternates that would use 64-bit flags, that apps with over 32 flags
(just dial, for now), could use, and use it only in dial, and revert
everything else back to normal?

Would that be more palatable?

murf



> (I would even claim that a dialplan app such as Dial(), with
> some 30 different one-letter options, is a recipe for disaster
> and misconfiguration, but that's another story...)
> 
> 	cheers
> 	luigi

-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3227 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20070718/fe441b84/attachment.bin