List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:May 28 2010 2:33am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)
Bug#52748
View as plain text  
Luís Soares wrote:
> Hi Zhenxing,
> 
>   Nice Work. 
>   Please find my review comments below.
> 
> STATUS
> ------
> 
>   Approved.
> 
> REQUIRED CHANGES
> ----------------
>  n/a
> 
> REQUESTS
> --------
>  n/a
> 
> SUGGESTIONS
> -----------
> 
>   S1. A simple 'grep strcpy plugin/semisync/*' showed me eight
>       lines of code. There is nothing wong in using strcpy if one
>       can claim that overflow is impossible. I trust this to be
>       the case for all eight lines.
> 
>       If in doubt (I confess I did not check thoroughly any of
>       the eight lines), please have a look at those lines.
> 

Thank you for the advice, I'll have a look and fix if necessary!

> DETAILS 
> -------
>  n/a
> 
> On Mon, 2010-05-24 at 02:57 +0000, He Zhenxing wrote:
> > #At file:///media/sdb2/hezx/work/mysql/bzr/b52748/trunk-bugfixing/ based on
> revid:jon.hauglid@stripped
> > 
> >  3057 He Zhenxing	2010-05-24
> >       BUG#52748 Semi-Sync ACK packet isn't check for length
> >       
> >       Check the length and use strncpy to make the code safer.
> > 
> >     modified:
> >       plugin/semisync/semisync_master.cc
> > === modified file 'plugin/semisync/semisync_master.cc'
> > --- a/plugin/semisync/semisync_master.cc	2010-03-11 02:22:18 +0000
> > +++ b/plugin/semisync/semisync_master.cc	2010-05-24 02:57:37 +0000
> > @@ -1048,6 +1048,7 @@ int ReplSemiSyncMaster::readSlaveReply(N
> >    const unsigned char *packet;
> >    char     log_file_name[FN_REFLEN];
> >    my_off_t log_file_pos;
> > +  ulong    log_file_len = 0;
> >    ulong    packet_len;
> >    int      result = -1;
> >  
> > @@ -1123,7 +1124,13 @@ int ReplSemiSyncMaster::readSlaveReply(N
> >    }
> >  
> >    log_file_pos = uint8korr(packet + REPLY_BINLOG_POS_OFFSET);
> > -  strcpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET);
> > +  log_file_len = packet_len - REPLY_BINLOG_NAME_OFFSET;
> > +  if (log_file_len > FN_REFLEN)
> > +  {
> > +    sql_print_error("Read semi-sync reply binlog file length too large");
> > +    goto l_end;
> > +  }
> > +  strncpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET,
> log_file_len);
> >  
> >    if (trc_level & kTraceDetail)
> >      sql_print_information("%s: Got reply (%s, %lu)",
> > 
> > text/bzr-bundle type attachment
> > (bzr/zhenxing.he@stripped)
> > # Bazaar merge directive format 2 (Bazaar 0.90)
> > # revision_id: zhenxing.he@stripped
> > # target_branch: file:///media/sdb2/hezx/work/mysql/bzr/b52748/trunk-\
> > #   bugfixing/
> > # testament_sha1: ec3ec5b0c6f878cf80c0045aba06ddf983f3d842
> > # timestamp: 2010-05-24 10:57:43 +0800
> > # base_revision_id: jon.hauglid@stripped\
> > #   60y19x3s9uua1rb6
> > # 
> > # Begin bundle
> > IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWRVpqI4AAb3fgEAQWP///3vv
> > 3jC////6UAQGuW3au87rnPDxcMiTEEnpplE/TTJimpmp6mIwE9T1B5A1MjJ6CUk1T9Ke00ZJ6U9R
> > 6g2UAAAGjRoAAAMiaJkyammSn6Cj0J6Q0AZGQ0ZDQDRkCSSYTQTRlHqbUn6mp6TT1AZGagBoAAAJ
> > SaTSGp+lHkajTR6Q9T0jQaZAaD0QBptQGlh/E1tgTv9hZlxu/6W7q+CBRcy2gER4ZGXPayCcgLrQ
> > myTX/duTZKiIWfLvLmwoBhxNgWBc8sRjr+PFXddumDI08hBT9obh12B+ttT/brdggjaknBaUq+CD
> > RfLuZeDhpUu8akcuzHeM2qVtHK9+DWvnwmgqCI85UTg5qpcuSdtL5i8kIhtAXa6Kb9V0UMxkzCNy
> > vYQ45hW6M5qRFPQfSK4KF3VNRVdRZoFKB9mVqkKPbzkI3yivuc2XJwq7GRQoXwrFHvxRPcB89IpP
> > pJSaqpD2TW14wKzhClEG9TMpqLDXQjiD2Gw0GUnA2Z2BnI3LCb6sR1jewjsmIuY2KqciD1nAyuS1
> > QU1piN/lREiKKl2NmFi2DGBPW1abOVFEBxbMSo4lVXG5mEIyoGKMTBpiOoEnaMGs0QgXqcxvxwbC
> > N9DjmM4Wg6s7AuZi19N6ALhcEhGTktrEetKxI0qrF1EXOI0UtEhQ6mR0xMSCMnII8GKlrahTMS7C
> > 4vjY6SQcI3pB5m4PUo5GEQIWJg4O6xom6xQdFELVS5LNT+RzShLOYLGhQeov2a4G+fkg9DQfJZf8
> > FcA+k0OkKyQilKyFSQgFjNhwuFw+wQfRUOI+O/3oM2b7AVgY0Y7l8HHCofqLVxFQWiktGUNjpgU5
> > VmKDkKmiLS2JynATpxTjGe/PF+zfvoVjVKMecPZUPNwp7jtbXUdRtwRlLWEi/6RFkFv7zZW7RAyz
> > iJXYqUS86o4H3Eq6RH/eG9SqY9+tqMkg8XJZjsNeagB9xxLHgAQjtdSEHDC/TauWmdMFkzmo3ao0
> > 4yMRHQh0DYUPAaCAqmeyVkUV99QtMYDyyDPm36pQy9w/2fjTCRXWwzinAwBiEsSDcIcipcSgugyh
> > GjAIm5fzbAlC8WSFKNYfimShFsTbptIDRu4EfoITvxT68N0KQimQtfp+GIv6JqlxLHxu2MtYZBWF
> > bg0VtyrSwpIoXKqr9ujbHFmfbus7Qhho2LJWHBcaha6YQHwZYNC1IYKWJ1fMVUBphTJzMIcts6Hv
> > cwMwmTIOPsvbYsqsBOBU5plm98ok2cDlwuVXH0Y4fFjiZlQR1TwyhUfUHVHSiSJUDv8RYi6W+odZ
> > O6RuwLNogIPQ+QrKr5O+A6kqNqXd0BLRxI7dPhTAxAoO0kLOIEIZu10mQLuD00lTdQcG+VEstOYK
> > wiBQoEZ6muRx2WngttSJtXR5VpVgrw32rcKKggugoq+zkGPIKC33I6gdxFEip8LxeEX61A6MVI9x
> > dyRThQkBVpqI4A==
> > 
> > 
> 


Thread
bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057) Bug#52748He Zhenxing24 May
  • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748Luís Soares27 May
    • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748He Zhenxing28 May
  • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748Alfranio Correia31 May
    • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748He Zhenxing31 May
      • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748Alfranio Correia31 May
        • Re: bzr commit into mysql-trunk-bugfixing branch (zhenxing.he:3057)Bug#52748He Zhenxing31 May