29 Oct
2017
29 Oct
'17
1:36 p.m.
Currently core/flags.h defines
typedef unsigned int flag_t;
It would better to use unsigned long in order to allow more that 32 flags on 64 bit
architectures.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288
30 Oct
30 Oct
11:51 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
This has implications on other parts, such as variables where they can be accessed and set
via $mf (iirc).
On the other hand, I don't think having a variable with different lengths is a good
approach, because it can break easily config behaviour when moving from one server to
another. It should be same size on 32b or 64b, so if a change to increase the size is to
be done, then should be `long long` to ensure it is 64 on all archs. An alternative would
be to make the field inside sip_msg_t an array of flag_t values, so can be one or more,
even beyond 64b. But this change will have also impact in other parts, therefore would
need careful review.
As an alternative for now would be using an avp or xavps, and do bitwise operations (`|`
and `&`) to set or test.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340394835
noon
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes:
This has implications on other parts, such as
variables where they can
be accessed and set via $mf (iirc).
Other parts of the code should not assume anything about the actual
length of flag_t. If they do, that code needs to be fixed.
As an alternative for now would be using an avp or
xavps, and do
bitwise operations (`|` and `&`) to set or test.
Use of avps for storing bit flags is too slow.
-- Juha
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340397304
12:06 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
My proposal is to change flag_t to unsigned long long after master is
opened for new features. Then there is lots of time to fix code that
might get broken.
-- Juha
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340398676
2:38 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
I wish that changing a structure is something simple, but is not.
The main constraint is having the config interpreter able to deal only with integer
numbers as values (static or via variables), and flags field stay behind a variable.
But there could be also memory alignment problems when fields size changes.
Another way could be adding a new field, extended flags, with a different set of functions
to handle them like setflagx()/resetflagx(...)/isflagxset(...), so the current behaviour
is not affected at all. At this moment, from extensibility point of view I think using an
array (static size) is better than single value field. Initially it can be of uint32_t[2]
to give access to 64 more flags, but in the future it can be changed if needed. The
functions will take two parameters, bit index and array index.
Just some ideas, it can be another way, but I would prefer not to affect that much
existing code...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340431612
8:44 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes:
Another way could be adding a new field, extended
flags, with a
different set of functions to handle them like
setflagx()/resetflagx(...)/isflagxset(...), so the current behaviour
is not affected at all.
That would be fine with me.
view I think using an array (static size) is better
than single value
field. Initially it can be of uint32_t[2] to give access to 64 more
flags, but in the future it can be changed if needed. The functions
will take two parameters, bit index and array index.
That would be too complex to use. A flag should be a single integer.
-- Juha
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340545188
31 Oct
31 Oct
7:19 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
How about using an array of unsigned ints, but functions have only one parameter? Array
index is param / 32 and flag number at the index is param % 32.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-340836695
14 Mar
14 Mar
2:06 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Just added support for extended flags (xflags) with a capacity of 64 flags. The functions
to manage them are exported by corex module. No time to test them, so feedback if all is
fine or not is appreciated.
I will also add t_fush_xflags() soon in tmx module.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-372998360
20 Mar
20 Mar
2:41 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes:
Just added support for extended flags (xflags) with a
capacity of 64
flags. The functions to manage them are exported by corex module. No
time to test them, so feedback if all is fine or not is appreciated.
I was going to give corex a try, but loading of the module failed like
this:
ar 21 01:39:35 trout sip-proxy[13013]: 0(13064) ERROR: <core>
[core/sr_module.c:582]: load_module(): could not open module
</usr/lib/x86_64-linux-gnu/sip-proxy/modules/corex.so>:
/usr/lib/x86_64-linux-gnu/sip-proxy/modules/corex.so: undefined symbol: setxflag
-- Juha
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-374583220
6:30 p.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
The functions were added by mistake inside a disabled ifdef zone. It should be fixed now
in master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-374664629
5 Apr
5 Apr
9:09 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closed #1288.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#event-1557989344
9:09 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closing, xflags being implemented.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378830173
9:29 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
As long as xflags do not work the same way as flags, I do not consider this issue closed.
Flags do not require call of t_flush_flags() after t_newtrans() in order to make flags
available in failure and onreply routes, but xflags does require call of t_flush_xflags().
This makes xflags cumbersome to use, since I would need to make t_flush_xflags() call
before every t_relay() call.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378833596
9:29 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Reopened #1288.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#event-1558010774
9:36 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
As I wrote on the mailing list, flags should not be migrated to the transaction after
t_newtran() unless one uses t_flush_flags() -- that was the desired functionality,
otherwise t_flush_flags() has no purpose.
You opened another item on this tracker related to this unexpected behaviour (#1490), and
I think it is better to sort this out there, because that is the unexpected behaviour.
xflags behave as expected. Whatever will be the conclusion of #1490 will be applied to
both flags and xflags, but makes no sense to track an issue with two separate items.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378834818
9:55 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes:
As I wrote on the mailing list, flags should not be
migrated to the
transaction after t_newtran() unless one uses t_flush_flags() -- that
was the desired functionality, otherwise t_flush_flags() has no
purpose.
I have used flags for many years and for me it has been desirable that
they are available after t_newtrans() in failure and onreply routes
without a need to call t_flush_flags(). If someone wants to change that
behavior and make t_flush_flags() purposeful, then a tm variable can be
introduced to achieve that new behavior.
You opened another item on this tracker related to
this unexpected
behaviour (#1490), and I think it is better to sort this out there,
because that is the unexpected behaviour. xflags behave as
expected. Whatever will be the conclusion of #1490 will be applied to
both flags and xflags, but makes no sense to track an issue with two
separate items.
I opened this issue, because I needed more that 32 flags, i.e., flags
that work the same way as before, but more of them. It is OK to replace
flags with xflags if my current configuration file keeps on working when
I textually change current flag function calls with xflag function
calls, but it is not OK if I need to start adding t_flush_xflags() calls
to it.
Issue "Is t_flush_flags() really needed? #1490" asks about the purpose
of t_flush_flags() function. I have not been able to figure out one,
but perhaps there is some. If there is no purpose, then the function
should be removed.
-- Juha
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378838094
10:27 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
The purpose of that t_flush_flags() is documented:
* https://www.kamailio.org/docs/modules/stable/modules/tmx.html#tmx.f.t_flush…
The role was to push the flags set after t_newtran() to transaction. If that is not
happening/is not needed for what so ever reason, needs to be sorted out in a way or
another.
The behaviour can stay as it is, with or without any new parameter -- this is going to be
discussed. But again, it doesn't belong here, the issue we discuss is with flushing
the flags after t_newtrans() and should be done in a single place. The xflags will follow
whatever is decided for t_flush_flags(), but right now xflags works as it is expected
based on docs for t_flush_flags().
Let's discuss further on #1490 and get the two work the same in a properly documented
behaviour.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378844680
10:55 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes:
The purpose of that t_flush_flags() is documented:
* https://www.kamailio.org/docs/modules/stable/modules/tmx.html#tmx.f.t_flush…
The role was to push the flags set after t_newtran() to
transaction. If that is not happening/is not needed for what so ever
reason, needs to be sorted out in a way or another.
Yes, and that is the topic of #1490.
The behaviour can stay as it is, with or without any
new parameter --
this is going to be discussed. But again, it doesn't belong here, the
issue we discuss is with flushing the flags after t_newtrans() and
should be done in a single place. The xflags will follow whatever is
decided for t_flush_flags(), but right now xflags works as it is
expected based on docs for t_flush_flags().
Let's discuss further on #1490 and get the two
work the same in a
properly documented behaviour.
OK, I'll close this issue with the note that the current semantics of
xflags does not correspond to that of flags.
-- Juha
10:58 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closing the issue even when xflags as currently implemented do not behave the same way as
flags and thus cannot be used as their replacement.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#issuecomment-378851744
10:58 a.m.
New subject: [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closed #1288.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/1288#event-1558135695
2453
days inactive
2611
days old
19 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Juha Heinanen -
juha-h