BUG#48993: valgrind errors in mysqlbinlog

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.
 

1 files changed, 8 insertions, 3 deletions

diff --git a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc
index 8b78fc252fe..ada22f19a65 100644
--- a/client/mysqlbinlog.cc
+++ b/client/mysqlbinlog.cc
@@ -832,7 +832,11 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
       print_event_info->common_header_len=
         glob_description_event->common_header_len;
       ev->print(result_file, print_event_info);
-      ev->temp_buf= 0; // as the event ref is zeroed
+      if (!remote_opt)
+        ev->free_temp_buf(); // free memory allocated in dump_local_log_entries
+      else
+        // disassociate but not free dump_remote_log_entries time memory
+        ev->temp_buf= 0;
       /*
         We don't want this event to be deleted now, so let's hide it (I
         (Guilhem) should later see if this triggers a non-serious Valgrind
@@ -1362,7 +1366,6 @@ static int parse_args(int *argc, char*** argv)
   int ho_error;
 
   result_file = stdout;
-  load_defaults("my",load_default_groups,argc,argv);
   if ((ho_error=handle_options(argc, argv, my_long_options, get_one_option)))
     exit(ho_error);
   if (debug_info_flag)
@@ -2019,8 +2022,10 @@ int main(int argc, char** argv)
 
   my_init_time(); // for time functions
 
+  if (load_defaults("my", load_default_groups, &argc, &argv))
+    exit(1);
+  defaults_argv= argv;
   parse_args(&argc, (char***)&argv);
-  defaults_argv=argv;
 
   if (!argc)
   {