Hello, @henningw ,
Tthank you for your comment. I do really appreciate it.
I must disagree on this :
"But there might be scenarios were you don't want this rehashing done "
YES, and that is why my proposal does not change current behavior.
With no changes in config file, nothing changes.
Closed #2363.
--
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/pull/2363#event-3530500165___
Kamailio (SER) - Development Mailing List
There are several misunderstanding of the code
As I mentioned, I didn't find a function to do hash natively over an integer,
so I used a string. I also asked, if there was any I could use, but received no
answer. I guess std::hash is out of the question.
It's wrong to consider that hash will
@Kalki70 - I think you start crossing the common sense line with your last
comment on a personal attack. As I said my non-technical remark was more from
an admiration point of view -- bugs that are found in code older than 15 years
deserve somehow an "highlighting prize", but it is not the case
@Kalki70 First of all thanks for the analysis (especially done in #2361). We
are a group of developers in this project and certainly are open to discussions
and feedback that is focussed at the topic.
I agree that the observed behaviour might be in some cases not that one want
and another
After careful consideration, I have decided not to implement a new algorithm.
My proposal is an enhancement of current hash algorithms. It's an extension.
Without changes it will behave like it has always been, since 2004. **It does
not break anything**.
However, if the user wants, via
@Kalki70 pushed 1 commit.
33bdabc77905b6b056022fa7eec560c0163f09ef Default mode is fully compatible with
previous implementation. Default mode "Doesn nothing"
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
You said "I am trying to FIX the current implementation", which implies there
is a problem, a bug. Initial developer maybe thought about more that you think,
just had to implement based on the requirements at that moment and they may
still be valid for specific use cases even after 16 years,
I never said there was a bug. I don't think it is a bug. For me a bug is
something wrong in the code, so the application does not behave as it is
expected. In this case the code is right. It does what it is expected to do.
Problem is, I think the expected behavior is a bad behavior. Maybe the
I didn't want to be sarcastic at all, just admired how you put your perspective
that you fix a bug -- you are right and others are wrong. And yes, I am sure I
am not aware of all SIP scenarios, every day I am surprised on what new issues
I have to troubleshoot on SIP traffic. That's why I
My fix does not break anything, as the re-hashing can be disabled by config.
Backward compatibility is a must, always, in my opinion. I did a mistake
though. Default value, if variable is not set, should be 0 ("disabled"). That
would be something to modify.
Maybe in August 2004 servers and
It is admirable that you are convinced it is a faulty implementation nobody
observed since August 2004 when the module was added and the hashing over
call-id was implemented, so you are going to fix it now for everyone.
Well, actually that is the wanted behaviour of the respective hashing
My comments in between, with [Kalki70]:
> First, the variables must be declared at the beginning of the
> functions/blocks, to be coherent with the rest of the code.
>
[Kalki70] I did not know you had this convention. I am more used to the
"declare the variable when you need it" approach, as it
First, the variables must be declared at the beginning of the functions/blocks,
to be coherent with the rest of the code.
Then, the new code is doing new-hashing over an integer converted to string,
which then do new-hashing over the returned index (again converted to string).
I do not like
@Kalki70 pushed 1 commit.
fdd623a50fedd4312f8324cf8796ef00eed12af5 dispatcher: Fixs rehash of has when
ds_rehash_max is 1.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
15 matches
Mail list logo