List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:December 24 2010 12:55pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469)
Bug#59123
View as plain text  
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;
> > >
> > >
> > >
> > >
> > >
> > 
> > 
> 
> 
> 


Thread
bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469) Bug#59123He Zhenxing23 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469)Bug#59123Alfranio Correia24 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469)Bug#59123He Zhenxing24 Dec
      • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469)Bug#59123He Zhenxing24 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3469)Bug#59123He Zhenxing27 Dec