He Zhenxing wrote:
> Hi Alfranio,
>
> Thank you for the review!
>
> Alfranio Correia wrote:
> > Hi Jasonh,
> >
> > STATUS
> > ------
> >
> > Patch not approved.
> >
> > REQUESTS
> > --------
> >
> > 1. You need to free the memory you have allocated.
> >
>
> I copied the code from Rows_query_log_event, but after thought on it, I
> think the my_alloc() is redundant because strmake will allocate memory
> in the thd, which will be automatically freed.
>
Sorry, please ignore this comment and comments below related to this, I
think I have mistaken strmake with thd->strmake.
> >
> > 2. If there is a failure, error is not propagated.
> >
>
> I think you mean the failure of my_alloc(), will have no problem after
> remove the call to my_alloc() as described above.
>
> >
> > 3. Can you point to the place in the code where the events
> > are null-terminated?
> >
>
> Log_event* Log_event::read_log_event(IO_CACHE* file,...)
> {
> ...
> if (!(buf = (char*) my_malloc(data_len+1, MYF(MY_WME))))
> {
> error = "Out of memory";
> goto err;
> }
> buf[data_len] = 0;
> ...
> }
>
> >
> > SUGGESTIONS
> > -----------
> >
> > 1. Please, consider replacing LEX_STRING by String.
> >
>
> After remove my_alloc(), I think use LEX_STRING is OK.
>
> >
> > 2. There are some typos in the commit message. Please, fix
> > them.
> >
>
> OK, will fix them
>
> >
> > Cheers.
> >
> > On 12/23/2010 07:01 AM, He Zhenxing wrote:
> > > #At file:///media/sdb2/hezx/work/mysql/bzr/test/trunk-bugfixing/ based on
> revid:zhenxing.he@stripped
> > >
> > > 3469 He Zhenxing 2010-12-23
> > > BUG#59123 rpl_stm_binlog_max_cache_size fails sporadically with
> found warnings
> > >
> > > rpl_stm_binlog_max_cache_size.test fails sporadically on PB2
> > > daily-trunk-bugfixing with binlog-checksum enabled, the expected
> > > incident error message was not properly suppressed, and the
> > > incident message would have extra strange characters appended.
> > >
> >
> > rpl_stm_binlog_max_cache_size.test fails sporadically on PB2
> > daily-trunk-bugfixing with binlog-checksum enabled. The expected
> > incident error message is not properly suppressed, and the
> > incident message has extra strange characters appended.
> >
> >
> > > The cause is that the incident error message restored from log
> event
> > > is not properly ended with '\0'. This does not cause problem
> normally
> > > because the error message is at the end of the event, and we added
> >
> > s/added/add/
> >
> > > a '\0' at the end of an event. but when binlog-checksum is enabled,
> > > the checksum value will be at the end after the message, and so
> > > extra strange characters could be shown and cause the suppression
> > > rule fail to suppress the message.
> > >
> >
> > s/at the end after/after
> >
> > s/rule fail to suppress the message/rule to fail/
> >
> > > Fixed the problem by allocate memory for the string and add '\0'
> > > at the end of the message.
> >
> > Fixed the problem by allocating memory for the string and
> > adding '\0' at the end of the message.
> >
> >
> > >
> > > modified:
> > > sql/log_event.cc
> > > === modified file 'sql/log_event.cc'
> > > --- a/sql/log_event.cc 2010-12-21 10:39:20 +0000
> > > +++ b/sql/log_event.cc 2010-12-23 07:01:33 +0000
> > > @@ -10335,7 +10335,9 @@ Incident_log_event::Incident_log_event(c
> > > uint8 len= 0; // Assignment to keep compiler happy
> > > const char *str= NULL; // Assignment to keep compiler happy
> > > read_str(&ptr, str_end,&str,&len);
> > > - m_message.str= const_cast<char*>(str);
> > > + if (!(m_message.str= (char*) my_malloc(len+1, MYF(MY_WME))))
> > > + DBUG_VOID_RETURN;
> > > + strmake(m_message.str, str, len);
> > > m_message.length= len;
> > > DBUG_PRINT("info", ("m_incident: %d", m_incident));
> > > DBUG_VOID_RETURN;
> > >
> > >
> > >
> > >
> > >
> >
> >
>
>
>
