Continued from issue: https://github.com/kamailio/kamailio/issues/1583
Say you have a kamailio config file with variables ordered as A(int),B(int),C(string),D(int) – (all belonging to the same group). Kamailio first creates a linked list of these variables in a given group and calculates the group size as follows [actual code](https://github.com/kamailio/kamailio/blob/master/src/core/cfg/cfg_script.c#L...)):
``` Pseudo-code --------------------------------------------------------------------------------------------- group->size = 0 group->vars = null // the pointer to the head of the linked list of variables create_group_linked_list: for variable in group: if variable is integer: round up group_size to the nearest 4 byte multiple group_size += sizeof(int) if variable is string: round up group_size to the nearest 8 byte multiple group_size += sizeof(str) //str is a kamailio type (size 16 in our C++ compiler) if group->vars is null: group->vars = variable else variables->next = group->vars group->vars = variable --------------------------------------------------------------------------------------------- Linked list creation
NULL group->size 0
A(int) group->size 4
B(int) -> A group->size 8
C(string) -> B -> A group->size 24
D(int) -> C -> B -> A group->size 28 ``` However, this linked list is not used as is. it is copied to a single shared memory block for all child processes. This is done in the [cfg_shmize()](https://github.com/kamailio/kamailio/blob/master/src/core/cfg/cfg_struct.c#L...) which further calls [cfg_script_fixup()](https://github.com/kamailio/kamailio/blob/master/src/core/cfg/cfg_script.c#L...) Basically, the function goes through the linked list of variables created above and calculates the offset of each variable in the memory block assuming that the offset of the first variable is 0. Following is the pseudo-code of what cfg_script_fixup() does for the linked list and offset calculation:
``` Pseudo-code -------------------------------------------------------------------------------------------- offset = 0 node = group->vars: while node != null: if node's value is int: round up offset to nearest multiple of 4 node->offset = offset offset += sizeof(int) if node's value is str: round up offset to nearest multiple of 8 node->offset = offset offset += sizeof(str) node = node -> next --------------------------------------------------------------------------------------------
linked list (from above): D(int) -> C(string) -> B(int) -> A(int)
offset=0
D D->offset = 0 offset = 4
C C->offset = 8 offset = 24
B B->offset = 24 offset = 28 A A->offset = 28 offset = 32 (which would be the actual group size) ```
So, as visible, **the order in which the linked list is being processed on the two cases is causing issues** (from A to D in one case and from D to A in the next case). while the group size calculation code block leads to a group size of 28, the group size according to the second code block would be 32 and the offset of the last variable would we 28. This would lead to the last variable (all 4 bytes) getting corrupted.
My temporary solution is to recalculate the group size every time a variable is added in the first code block by re-traversing the linked list but is definitely not the ideal solution.
Can you paste here the diff of your patch in order to review and see if there can be any improvement that can be done?
let me see what I can do :)
Hi,
thank you very much for the detailed analyzes and finding the root cause. I could verify the difference in the group sizes based on your example.
Could you please try the attached patch? It appends the new variable to the end of the group now to keep the order intact, and it also adds an extra sanity check.
Thanks, Miklos [cfg_script.diff.gz](https://github.com/kamailio/kamailio/files/2506182/cfg_script.diff.gz)
So, now the linked list is populated in the same order as it is read. This solved the issue. The diff looks good to me [tested].
Thanks for the help! :)
@vinesinha - thanks for detailed troubleshooting and testing!
@mtirpak - thanks for the patch, you can push the commit with it to master branch if you want, otherwise I can do it.
Thanks @miconda and @mtirpak for all the help!
Fixed with the patch 2ecf601c472bb81b9cf4ffd5b1ac17c4dfd742f2.
Closed #1679.