• Luis Soares's avatar
    BUG#48993: valgrind errors in mysqlbinlog · f0b38904
    Luis Soares authored
    I found three issues during the analysis:
     1. Memory leak caused by temp_buf not being freed;
     2. Memory leak caused when handling argv;
     3. Conditional jump that depended on unitialized values.
    
    Issue #1
    --------
    
      DESCRIPTION: when mysqlbinlog is reading from a remote location
      the event temp_buf references the incoming stream (in NET
      object), which is not freed by mysqlbinlog explicitly. On the
      other hand, when it is reading local binary log, it points to a
      temporary buffer that needs to be explicitly freed. For both
      cases, the temp_buf was not freed by mysqlbinlog, instead was
      set to 0.  This clearly disregards the free required in the
      second case, thence creating a memory leak.
    
      FIX: we make temp_buf to be conditionally freed depending on
      the value of remote_opt. Found out that similar fix is already
      in most recent codebases.
    
    Issue #2 
    --------
    
      DESCRIPTION: load_defaults is called by parse_args, and it
      reads default options from configuration files and put them
      BEFORE the arguments that are already in argc and argv. This is
      done resorting to MEM_ROOT. However, parse_args calls
      handle_options immediately after which changes argv. Later when
      freeing the defaults, pointers to MEM_ROOT won't match, causing
      the memory not to be freed:
    
      void free_defaults(char **argv)
      {
        MEM_ROOT ptr
        memcpy_fixed((char*) &ptr,(char *) argv - sizeof(ptr), sizeof(ptr));
        free_root(&ptr,MYF(0));
      }
    
      FIX: we remove load_defaults from parse_args and call it
      before. Then we save argv with defaults in defaults_argv BEFORE
      calling parse_args (which inside can then call handle_options
      at will). Actually, found out that this is in fact kind of a
      backport for BUG#38468 into 5.1, so I merged in the test case
      as well and added error check for load_defaults call.
    
      Fix based on:
      revid:zhenxing.he@sun.com-20091002081840-uv26f0flw4uvo33y
    
    
    Issue #3 
    --------
    
      DESCRIPTION: the structure st_print_event_info constructor
      would not initialize the sql_mode member, although it did for
      sql_mode_inited (set to false). This would later raise the
      warning in valgrind when printing the sql_mode in the event
      header, as this print out is protected by a check against
      sql_mode_inited and sql_mode variables. Given that sql_mode was
      not initialized valgrind would output the warning.
    
      FIX: we add initialization of sql_mode to the
      st_print_event_info constructor.
     
    
    client/mysqlbinlog.cc:
      - Conditionally free ev->temp_buf.
      - save defaults_argv before handle_options is called.
    mysql-test/t/mysqlbinlog.test:
      Added test case from BUG#38468.
    sql/log_event.cc:
      Added initialization of sql_mode for st_print_event_info.
    f0b38904
mysqlbinlog.cc 69.1 KB