Hi Guilhem,
I agree with all of your comments/answers. I just want to emphasize some
of them below or answer where I felt a question.
Guilhem Bichot, 27.11.2007 14:49:
...
> On Wed, Nov 21, 2007 at 10:21:28PM +0100, Ingo Strüwing wrote:
...
>> Guilhem Bichot, 06.11.2007 20:56:
>> ...
>>> ChangeSet@stripped, 2007-11-06 20:56:05+01:00, guilhem@stripped +59 -0
>>> WL#866 - Online backup driver for MyISAM.
...
> mmmm how about STATIC_CAST? My reasons:
> - there are static_cast, const_cast, reinterpret_cast, all used in
> MySQL (and dynamic_cast, unused)
> - if I introduce CAST(), some colleagues are maybe going to use it in
> their new C++ code, it would be some sort of regression, because I
> find it less explicit that static_cast (you have to remember that CAST
> is like static_cast).
No big deal. I agree.
...
>> Just to exclude any doubts what I mean: I want to tune the number of
>> bytes to write for 'filepos' and/or 'length' in a "small" log header to
>> keep the avarage header length as small as possible.
...
> How about 4 / 8 ?
Ok, ok. It was just an idea. Thank you for investigating.
...
> I meant that if post_write was called before doing the write, the
> physical logging algorithm would break (you remember, we must check
> physical_logging_state *after* doing the write).
Yes, that's absolutely clear. My concern was that you may over-stress
this trivial and obvious fact. I agree with the new comments.
...
>> You changed the indentation from 2 to 3. This violates our coding
>> guidelines. This does also apply to many function comments, but I don't
>> repeat this complaint ans more.
>
> It's not me, it's my xemacs :)
> Now I have a problem, as I wrote to dev-public last week-end: I use
> the recommended xemacs setting, but it puts 3 spaces on the line
> after /**. I have asked colleagues for help. Meanwhile, I fixed by
> hand all these occurences of 3 spaces that I could find, I hope I
> missed none.
Well, for me it's absolutely unimportant, how many spaces we use for
indentation. But when we specify some value, then everybody should stick
to it, regardless, which editor he uses.
I agree that 3 would be easier for (x)emacs users. I have no objections
against a change in the guidelines. The number didn't come from me
anyway. I would happily use a different for future patches. But please
announce any change in the guideline, if you do it.
...
>> I suggest to use head_len[command][big_numbers ? 1 : 0]. At all other
>> places, the exact value of 'big_numbers' doesn't matter.
>
> I changed to:
> command-= (big_numbers= (command & MI_LOG_BIG_NUMBERS));
> big_numbers= test(big_numbers);
> if (my_b_read(etc))
I agree. There is just a weak argument: I had 'test' in mind at the
first moment too. But then I thought it might be meant to return a
boolean. An array index needs an integer though. So I thought writing it
explicit would be slightly "cleaner".
But 'test' is acceptable for me. No need to change again.
...
Regards
Ingo
--
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Dachauer Str. 37, D-80335 München
Geschäftsführer: Kaj Arnö - HRB München 162140
