Sanja,
Re: https://mariadb.atlassian.net/browse/MDEV-4645
Re: http://lists.askmonty.org/pipermail/commits/2013-July/005093.html
I saw your commit on the commits list (sorry, wasn't on the list before the
mail went out, so I can't reply directly to that message).
A couple of points related to the commit:
- You seem to have an error in the get_header_len, as it's implemented
only in the base class, and you've left functions with the correct
signature for it but the wrong names in the two frozen classes (they were
left named has_fixed_minimal_header). This would mean the change is
completely broken. :)
- The change to get_header_len I think is in general fine, although it
does alter the point of the whole thing -- there is again no way to exactly
identify which events have frozen headers, programmatically (i.e. you can't
differentiate them from events who just "happen" to have header_len =
LOG_EVENT_MINIMAL_HEADER_LEN). I think that doesn't matter for this
patch, but does defeat the point of what I was trying to add.
- I don't really understand why you've made the hex printing code so
complex and completely unreadable (IMHO). This is not performance-sensitive
code and in my opinion should favor readability over performance. All you
seem to have saved in this change is to avoid printing some static
formatting characters ("#", "|", spaces, etc.) inside the loop. However, I
can hardly even figure out what it's supposed to be doing now.
- Additionally, you've changed the output slightly -- normally the ASCII
character section prints its ending "|" immediately after the last
character; it seems now to be fixed position (leaving extra spaces before
"|". This differs from the output of hexdump, and it somewhat
ambiguous/confusing.
- You've mixed tabs and spaces. :(
