Hi!
>>>>> "Benjamin" == Benjamin Pflugmann <benjamin-mysql@stripped>
> writes:
<cut>
>> +++ 1.15/mysys/mf_keycache.c Mon Nov 18 14:59:43 2002
>> @@ -548,13 +548,14 @@
>> count++;
>> }
>> /* Only allocate a new buffer if its bigger than the one we have */
>> - if (count <= FLUSH_CACHE ||
>> - !(cache=(SEC_LINK**) my_malloc(sizeof(SEC_LINK*)*count,MYF(0))))
>> + if (count > FLUSH_CACHE)
>> {
>> - cache=cache_buff; /* Fall back to safe buffer */
>> - count=FLUSH_CACHE;
>> + if (!(cache=(SEC_LINK**) my_malloc(sizeof(SEC_LINK*)*count,MYF(0))))
>> + {
>> + cache=cache_buff; /* Fall back to safe buffer */
>> + count=FLUSH_CACHE;
>> + }
>> }
>> - end=cache+count;
>> }
Benjamin> Sorry, if I am ignorant here, but before the change, it was assured,
Benjamin> that
Benjamin> count <= FLASH_CACHE
Benjamin> after this code part.
The old code was:
if (count <= FLUSH_CACHE ||
!(cache=(SEC_LINK**) my_malloc(sizeof(SEC_LINK*)*count,MYF(0))))
{
cache=cache_buff; /* Fall back to safe buffer */
count=FLUSH_CACHE;
}
Assume that count > FLUSH_CACHE
In this case there is two options:
- The malloc succeeds
Then it's ok that count contains a bigger value than FLUSH_CACHE
as the buffer is large enough to hold count values.
- If malloc fails, then count is set to FLUSH_CACHE and we are setting
cache to point to the stack allocated cache of FLUSH_CACHE size.
In other words, here count can be bigger than FLUSH_CACHE if the
malloc succeeds.
The 'tricky' part with the old code is that it also set
count to FLUSH_CACHE if count was < FLUSH_CACHE.
This was ok as the loop afterwards will stop as soon as the list is
exhausted, which will happen before count reaches FLUSH_CACHE.
This was not obvious so I changed the code to make it easier to read:
if (count > FLUSH_CACHE)
{
if (!(cache=(SEC_LINK**) my_malloc(sizeof(SEC_LINK*)*count,MYF(0))))
{
cache=cache_buff; /* Fall back to safe buffer */
count=FLUSH_CACHE;
}
}
Benjamin> Now, it may be left at a bigger value (if the
Benjamin> call to my_malloc fails).
If my_malloc() failed in the old case then count was set to FLUSH_CACHE.
In the new code we do exactly the same thing.
Did you misread the patch or did I miss something ?
<cut>
Benjamin> No long explanation needed, I just wanted to make you aware of this,
Benjamin> in case it is an issue.
Thanks for the concern but I belive the new code is ok. If not,
please respond to me where the above logic fails.
Regards,
Monty
