Sven, hello.
The patch is good.
Thanks for this big work!
cheers,
Andrei
> Below is the list of changes that have just been committed into a local
> 5.1 repository of sven. When sven does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2008-02-05 16:55:44+01:00, sven@riska.(none) +5 -0
> BUG#33247: mysqlbinlog does not clean up after itself on abnormal termination
> Problem: mysqlbinlog does not free memory if an error happens.
> Fix: binlog-processing functions do not call exit() anymore. Instead, they
> print an error and return an error code. Error codes are propagated all
> the way back to main, and all allocated memory is freed on the way.
>
> client/mysqlbinlog.cc@stripped, 2008-02-05 16:55:41+01:00, sven@riska.(none) +648
> -398
> - New error handling policy: functions processing binlogs don't just
> exit() anymore. Instead, they print a message and return an error
> status.
> - New policy for the global `mysql' and `glob_description_event': these
> are not passed as parameters anymore. The global pointer is used
> instead.
> - More error situations are detected and reported.
> - Better error messages: the program never terminates with exit status 1
> without explanation any more. Fixed spelling errors. Use consistent
> format of messages (a single line beginning with "ERROR: " or
> "WARNING: " and ending with "." is printed to stderr.)
> - New memory handling: memory is always freed on program termination.
> - Better comments: more functions are explained, doxygen is used, and
> more precise formulations in some existing comments.
>
> mysql-test/suite/binlog/r/binlog_base64_flag.result@stripped, 2008-02-05 16:55:41+01:00,
> sven@riska.(none) +6 -1
> Result file updated since output format of mysqlbinlog changed while the
> test was disabled.
>
> mysql-test/suite/binlog/t/binlog_killed.test@stripped, 2008-02-05 16:55:41+01:00,
> sven@riska.(none) +3 -3
> Mysqlbinlog now works as described when the binlog is open. Hence, the
> --force-if-open flag must be passed
>
> mysql-test/suite/binlog/t/binlog_killed_simulate.test@stripped, 2008-02-05
> 16:55:41+01:00, sven@riska.(none) +2 -2
> Mysqlbinlog now works as described when the binlog is open. Hence, the
> --force-if-open flag must be passed
>
> mysql-test/suite/binlog/t/disabled.def@stripped, 2008-02-05 16:55:41+01:00,
> sven@riska.(none) +0 -1
> Now that mysqlbinlog cleans up after itself on abnormal termination, we
> can enable this test again.
>
> diff -Nrup a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc
> --- a/client/mysqlbinlog.cc 2007-12-14 19:01:58 +01:00
> +++ b/client/mysqlbinlog.cc 2008-02-05 16:55:41 +01:00
> @@ -59,7 +59,8 @@ static const char* default_dbug_option =
> #endif
> static const char *load_default_groups[]= { "mysqlbinlog","client",0 };
>
> -void sql_print_error(const char *format, ...);
> +static void error(const char *format, ...) ATTRIBUTE_FORMAT(printf, 1, 2);
> +static void warning(const char *format, ...) ATTRIBUTE_FORMAT(printf, 1, 2);
>
> static bool one_database=0, to_last_remote_log= 0, disable_log_bin= 0;
> static bool opt_hexdump= 0;
> @@ -92,24 +93,33 @@ static ulonglong rec_count= 0;
> static short binlog_flags = 0;
> static MYSQL* mysql = NULL;
> static const char* dirname_for_local_load= 0;
> -static bool stop_passed= 0;
> -static my_bool file_not_closed_error= 0;
>
> -/*
> - check_header() will set the pointer below.
> - Why do we need here a pointer on an event instead of an event ?
> - This is because the event will be created (alloced) in read_log_event()
> - (which returns a pointer) in check_header().
> -*/
> -static Format_description_log_event* glob_description_event;
> -
> -static int dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
> - const char* logname);
> -static int dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
> - const char* logname);
> -static int dump_log_entries(const char* logname);
> -static void die(const char* fmt, ...) __attribute__ ((__noreturn__));
> -static MYSQL* safe_connect();
> +/**
> + Pointer to the Format_description_log_event of the currently active binlog.
> +
> + This will be changed each time a new Format_description_log_event is
> + found in the binlog. It is finally destroyed at program termination.
> +*/
> +static Format_description_log_event* glob_description_event= NULL;
> +
> +/**
> + Exit status for functions in this file.
> +*/
> +enum Exit_status {
> + /** No error occurred and execution should continue. */
> + OK_CONTINUE= 0,
> + /** An error occurred and execution should stop. */
> + ERROR_STOP,
> + /** No error occurred but execution should stop. */
> + OK_STOP
> +};
> +
> +static Exit_status dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
> + const char* logname);
> +static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
> + const char* logname);
> +static Exit_status dump_log_entries(const char* logname);
> +static Exit_status safe_connect();
>
>
> class Load_log_processor
> @@ -132,22 +142,29 @@ class Load_log_processor
> char *fname;
> Create_file_log_event *event;
> };
> + /*
> + @todo Should be a map (e.g., a hash map), not an array. With the
> + present implementation, the number of elements in this array is
> + about the number of files loaded since the server started, which
> + may be big after a few years. We should be able to use existing
> + library data structures for this. /Sven
> + */
> DYNAMIC_ARRAY file_names;
>
> - /*
> - Looking for new uniquie filename that doesn't exist yet by
> - adding postfix -%x
> + /**
> + Looks for a non-existing filename by adding a numerical suffix to
> + the given base name, creates the generated file, and returns the
> + filename by modifying the filename argument.
> +
> + @param[in,out] filename Base filename
> +
> + @param[in,out] file_name_end Pointer to last character of
> + filename. The numerical suffix will be written to this position.
> + Note that there must be a least five bytes of allocated memory
> + after file_name_end.
>
> - SYNOPSIS
> - create_unique_file()
> -
> - filename buffer for filename
> - file_name_end tail of buffer that should be changed
> - should point to a memory enough to printf("-%x",..)
> -
> - RETURN VALUES
> - values less than 0 - can't find new filename
> - values great or equal 0 - created file with found filename
> + @retval -1 Error (can't find new filename).
> + @retval >=0 Found file.
> */
> File create_unique_file(char *filename, char *file_name_end)
> {
> @@ -201,22 +218,20 @@ public:
> delete_dynamic(&file_names);
> }
>
> - /*
> - Obtain Create_file event for LOAD DATA statement by its file_id.
> + /**
> + Obtain Create_file event for LOAD DATA statement by its file_id
> + and remove it from this Load_log_processor's list of events.
> +
> + Checks whether we have already seen a Create_file_log_event with
> + the given file_id. If yes, returns a pointer to the event and
> + removes the event from array describing active temporary files.
> + From this moment, the caller is responsible for freeing the memory
> + occupied by the event.
>
> - SYNOPSIS
> - grab_event()
> - file_id - file_id identifiying LOAD DATA statement
> -
> - DESCRIPTION
> - Checks whenever we have already seen Create_file event for this file_id.
> - If yes then returns pointer to it and removes it from array describing
> - active temporary files. Since this moment caller is responsible for
> - freeing memory occupied by this event and associated file name.
> -
> - RETURN VALUES
> - Pointer to Create_file event or 0 if there was no such event
> - with this file_id.
> + @param[in] file_id File id identifying LOAD DATA statement.
> +
> + @return Pointer to Create_file_log_event, or NULL if we have not
> + seen any Create_file_log_event with this file_id.
> */
> Create_file_log_event *grab_event(uint file_id)
> {
> @@ -231,23 +246,20 @@ public:
> return res;
> }
>
> - /*
> - Obtain file name of temporary file for LOAD DATA statement by its file_id.
> + /**
> + Obtain file name of temporary file for LOAD DATA statement by its
> + file_id and remove it from this Load_log_processor's list of events.
> +
> + @param[in] file_id Identifier for the LOAD DATA statement.
> +
> + Checks whether we have already seen Begin_load_query event for
> + this file_id. If yes, returns the file name of the corresponding
> + temporary file and removes the filename from the array of active
> + temporary files. From this moment, the caller is responsible for
> + freeing the memory occupied by this name.
>
> - SYNOPSIS
> - grab_fname()
> - file_id - file_id identifiying LOAD DATA statement
> -
> - DESCRIPTION
> - Checks whenever we have already seen Begin_load_query event for this
> - file_id. If yes then returns file name of corresponding temporary file.
> - Removes record about this file from the array of active temporary files.
> - Since this moment caller is responsible for freeing memory occupied by
> - this name.
> -
> - RETURN VALUES
> - String with name of temporary file or 0 if we have not seen Begin_load_query
> - event with this file_id.
> + @return String with the name of the temporary file, or NULL if we
> + have not seen any Begin_load_query_event with this file_id.
> */
> char *grab_fname(uint file_id)
> {
> @@ -264,19 +276,29 @@ public:
> }
> return res;
> }
> - int process(Create_file_log_event *ce);
> - int process(Begin_load_query_log_event *ce);
> - int process(Append_block_log_event *ae);
> + Exit_status process(Create_file_log_event *ce);
> + Exit_status process(Begin_load_query_log_event *ce);
> + Exit_status process(Append_block_log_event *ae);
> File prepare_new_file_for_old_format(Load_log_event *le, char *filename);
> - int load_old_format_file(NET* net, const char *server_fname,
> - uint server_fname_len, File file);
> - int process_first_event(const char *bname, uint blen, const char *block,
> - uint block_len, uint file_id,
> - Create_file_log_event *ce);
> + Exit_status load_old_format_file(NET* net, const char *server_fname,
> + uint server_fname_len, File file);
> + Exit_status process_first_event(const char *bname, uint blen,
> + const char *block,
> + uint block_len, uint file_id,
> + Create_file_log_event *ce);
> };
>
>
> +/**
> + Creates and opens a new temporary file in the directory specified by previous call
> to init_by_dir_name() or init_by_cur_dir().
> +
> + @param[in] le The basename of the created file will start with the
> + basename of the file pointed to by this Load_log_event.
> +
> + @param[out] filename Buffer to save the filename in.
>
> + @return File handle >= 0 on success, -1 on error.
> +*/
> File Load_log_processor::prepare_new_file_for_old_format(Load_log_event *le,
> char *filename)
> {
> @@ -284,13 +306,13 @@ File Load_log_processor::prepare_new_fil
> char *tail;
> File file;
>
> - fn_format(filename, le->fname, target_dir_name, "", 1);
> + fn_format(filename, le->fname, target_dir_name, "", MY_REPLACE_DIR);
> len= strlen(filename);
> tail= filename + len;
>
> if ((file= create_unique_file(filename,tail)) < 0)
> {
> - sql_print_error("Could not construct local filename %s",filename);
> + error("Could not construct local filename %s.",filename);
> return -1;
> }
>
> @@ -300,16 +322,33 @@ File Load_log_processor::prepare_new_fil
> }
>
>
> -int Load_log_processor::load_old_format_file(NET* net, const char*server_fname,
> - uint server_fname_len, File file)
> +/**
> + Reads a file from a server and saves it locally.
> +
> + @param[in,out] net The server to read from.
> +
> + @param[in] server_fname The name of the file that the server should
> + read.
> +
> + @param[in] server_fname_len The length of server_fname.
> +
> + @param[in,out] file The file to write to.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +Exit_status Load_log_processor::load_old_format_file(NET* net,
> + const char*server_fname,
> + uint server_fname_len,
> + File file)
> {
> uchar buf[FN_REFLEN+1];
> buf[0] = 0;
> memcpy(buf + 1, server_fname, server_fname_len + 1);
> if (my_net_write(net, buf, server_fname_len +2) || net_flush(net))
> {
> - sql_print_error("Failed requesting the remote dump of %s", server_fname);
> - return -1;
> + error("Failed requesting the remote dump of %s.", server_fname);
> + return ERROR_STOP;
> }
>
> for (;;)
> @@ -319,8 +358,8 @@ int Load_log_processor::load_old_format_
> {
> if (my_net_write(net, (uchar*) "", 0) || net_flush(net))
> {
> - sql_print_error("Failed sending the ack packet");
> - return -1;
> + error("Failed sending the ack packet.");
> + return ERROR_STOP;
> }
> /*
> we just need to send something, as the server will read but
> @@ -331,63 +370,63 @@ int Load_log_processor::load_old_format_
> }
> else if (packet_len == packet_error)
> {
> - sql_print_error("Failed reading a packet during the dump of %s ",
> - server_fname);
> - return -1;
> + error("Failed reading a packet during the dump of %s.", server_fname);
> + return ERROR_STOP;
> }
>
> if (packet_len > UINT_MAX)
> {
> - sql_print_error("Illegal length of packet read from net");
> - return -1;
> + error("Illegal length of packet read from net.");
> + return ERROR_STOP;
> }
> if (my_write(file, (uchar*) net->read_pos,
> (uint) packet_len, MYF(MY_WME|MY_NABP)))
> - return -1;
> + return ERROR_STOP;
> }
>
> - return 0;
> + return OK_CONTINUE;
> }
>
>
> -/*
> - Process first event in the sequence of events representing LOAD DATA
> - statement.
> -
> - SYNOPSIS
> - process_first_event()
> - bname - base name for temporary file to be created
> - blen - base name length
> - block - first block of data to be loaded
> - block_len - first block length
> - file_id - identifies LOAD DATA statement
> - ce - pointer to Create_file event object if we are processing
> - this type of event.
> -
> - DESCRIPTION
> - Creates temporary file to be used in LOAD DATA and writes first block of
> - data to it. Registers its file name (and optional Create_file event)
> - in the array of active temporary files.
> -
> - RETURN VALUES
> - 0 - success
> - non-0 - error
> -*/
> -
> -int Load_log_processor::process_first_event(const char *bname, uint blen,
> - const char *block, uint block_len,
> - uint file_id,
> - Create_file_log_event *ce)
> +/**
> + Process the first event in the sequence of events representing a
> + LOAD DATA statement.
> +
> + Creates a temporary file to be used in LOAD DATA and writes first
> + block of data to it. Registers its file name (and optional
> + Create_file event) in the array of active temporary files.
> +
> + @param bname Base name for temporary file to be created.
> + @param blen Base name length.
> + @param block First block of data to be loaded.
> + @param block_len First block length.
> + @param file_id Identifies the LOAD DATA statement.
> + @param ce Pointer to Create_file event object if we are processing
> + this type of event.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +Exit_status Load_log_processor::process_first_event(const char *bname,
> + uint blen,
> + const char *block,
> + uint block_len,
> + uint file_id,
> + Create_file_log_event *ce)
> {
> uint full_len= target_dir_name_len + blen + 9 + 9 + 1;
> - int error= 0;
> + Exit_status retval= OK_CONTINUE;
> char *fname, *ptr;
> File file;
> File_name_record rec;
> DBUG_ENTER("Load_log_processor::process_first_event");
>
> if (!(fname= (char*) my_malloc(full_len,MYF(MY_WME))))
> - DBUG_RETURN(-1);
> + {
> + error("Out of memory.");
> + delete ce;
> + DBUG_RETURN(ERROR_STOP);
> + }
>
> memcpy(fname, target_dir_name, target_dir_name_len);
> ptr= fname + target_dir_name_len;
> @@ -397,9 +436,10 @@ int Load_log_processor::process_first_ev
>
> if ((file= create_unique_file(fname,ptr)) < 0)
> {
> - sql_print_error("Could not construct local filename %s%s",
> - target_dir_name,bname);
> - DBUG_RETURN(-1);
> + error("Could not construct local filename %s%s.",
> + target_dir_name,bname);
> + delete ce;
> + DBUG_RETURN(ERROR_STOP);
> }
>
> rec.fname= fname;
> @@ -407,23 +447,39 @@ int Load_log_processor::process_first_ev
>
> if (set_dynamic(&file_names, (uchar*)&rec, file_id))
> {
> - sql_print_error("Could not construct local filename %s%s",
> - target_dir_name, bname);
> - DBUG_RETURN(-1);
> + error("Out of memory.");
> + delete ce;
> + DBUG_RETURN(ERROR_STOP);
> }
>
> if (ce)
> ce->set_fname_outside_temp_buf(fname, strlen(fname));
>
> if (my_write(file, (uchar*)block, block_len, MYF(MY_WME|MY_NABP)))
> - error= -1;
> + {
> + error("Failed writing to file.");
> + retval= ERROR_STOP;
> + }
> if (my_close(file, MYF(MY_WME)))
> - error= -1;
> - DBUG_RETURN(error);
> + {
> + error("Failed closing file.");
> + retval= ERROR_STOP;
> + }
> + DBUG_RETURN(retval);
> }
>
>
> -int Load_log_processor::process(Create_file_log_event *ce)
> +/**
> + Process the given Create_file_log_event.
> +
> + @see Load_log_processor::process_first_event(const char*,uint,const
> char*,uint,uint,Create_file_log_event*)
> +
> + @param ce Create_file_log_event to process.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +Exit_status Load_log_processor::process(Create_file_log_event *ce)
> {
> const char *bname= ce->fname + dirname_length(ce->fname);
> uint blen= ce->fname_len - (bname-ce->fname);
> @@ -433,14 +489,46 @@ int Load_log_processor::process(Create_f
> }
>
>
> -int Load_log_processor::process(Begin_load_query_log_event *blqe)
> +/**
> + Process the given Begin_load_query_log_event.
> +
> + @see Load_log_processor::process_first_event(const char*,uint,const
> char*,uint,uint,Create_file_log_event*)
> +
> + @param ce Begin_load_query_log_event to process.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +Exit_status Load_log_processor::process(Begin_load_query_log_event *blqe)
> {
> return process_first_event("SQL_LOAD_MB", 11, blqe->block, blqe->block_len,
> blqe->file_id, 0);
> }
>
>
> -int Load_log_processor::process(Append_block_log_event *ae)
> +/**
> + Process the given Append_block_log_event.
> +
> + Appends the chunk of the file contents specified by the event to the
> + file created by a previous Begin_load_query_log_event or
> + Create_file_log_event.
> +
> + If the file_id for the event does not correspond to any file
> + previously registered through a Begin_load_query_log_event or
> + Create_file_log_event, this member function will print a warning and
> + return OK_CONTINUE. It is safe to return OK_CONTINUE, because no
> + query will be written for this event. We should not print an error
> + and fail, since the missing file_id could be because a (valid)
> + --start-position has been specified after the Begin/Create event but
> + before this Append event.
> +
> + @param ae Append_block_log_event to process.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> +
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +Exit_status Load_log_processor::process(Append_block_log_event *ae)
> {
> DBUG_ENTER("Load_log_processor::process");
> const char* fname= ((ae->file_id < file_names.elements) ?
> @@ -450,15 +538,24 @@ int Load_log_processor::process(Append_b
> if (fname)
> {
> File file;
> - int error= 0;
> + Exit_status retval= OK_CONTINUE;
> if (((file= my_open(fname,
> O_APPEND|O_BINARY|O_WRONLY,MYF(MY_WME))) < 0))
> - DBUG_RETURN(-1);
> + {
> + error("Failed opening file %s", fname);
> + DBUG_RETURN(ERROR_STOP);
> + }
> if (my_write(file,(uchar*)ae->block,ae->block_len,MYF(MY_WME|MY_NABP)))
> - error= -1;
> + {
> + error("Failed writing to file %s", fname);
> + retval= ERROR_STOP;
> + }
> if (my_close(file,MYF(MY_WME)))
> - error= -1;
> - DBUG_RETURN(error);
> + {
> + error("Failed closing file %s", fname);
> + retval= ERROR_STOP;
> + }
> + DBUG_RETURN(retval);
> }
>
> /*
> @@ -466,16 +563,25 @@ int Load_log_processor::process(Append_b
> --start-position). Assuming it's a big --start-position, we just do
> nothing and print a warning.
> */
> - fprintf(stderr,"Warning: ignoring Append_block as there is no \
> -Create_file event for file_id: %u\n",ae->file_id);
> - DBUG_RETURN(-1);
> + warning("Ignoring Append_block as there is no "
> + "Create_file event for file_id: %u", ae->file_id);
> + DBUG_RETURN(OK_CONTINUE);
> }
>
>
> -Load_log_processor load_processor;
> +static Load_log_processor load_processor;
> +
>
> +/**
> + Indicates whether the given database should be filtered out,
> + according to the --database=X option.
> +
> + @param log_dbname Name of database.
>
> -static bool check_database(const char *log_dbname)
> + @return nonzero if the database with the given name should be
> + filtered out, 0 otherwise.
> +*/
> +static bool shall_skip_database(const char *log_dbname)
> {
> return one_database &&
> (log_dbname != NULL) &&
> @@ -483,8 +589,23 @@ static bool check_database(const char *l
> }
>
>
> +/**
> + Prints the given event in base64 format.
>
> -static int
> + The header is printed to the head cache and the body is printed to
> + the body cache of the print_event_info structure. This allows all
> + base64 events corresponding to the same statement to be joined into
> + one BINLOG statement.
> +
> + @param[in] ev Log_event to print.
> + @param[in,out] result_file FILE to which the output will be written.
> + @param[in,out] print_event_info Parameters and context state
> + determining how to print.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +static Exit_status
> write_event_header_and_base64(Log_event *ev, FILE *result_file,
> PRINT_EVENT_INFO *print_event_info)
> {
> @@ -497,35 +618,44 @@ write_event_header_and_base64(Log_event
> ev->print_base64(body, print_event_info, FALSE);
>
> /* Read data from cache and write to result file */
> - DBUG_RETURN(copy_event_cache_to_file_and_reinit(head, result_file) ||
> - copy_event_cache_to_file_and_reinit(body, result_file));
> + if (copy_event_cache_to_file_and_reinit(head, result_file) ||
> + copy_event_cache_to_file_and_reinit(body, result_file))
> + {
> + error("Error writing event to file.");
> + DBUG_RETURN(ERROR_STOP);
> + }
> + DBUG_RETURN(OK_CONTINUE);
> }
>
>
> -/*
> - Process an event
> -
> - SYNOPSIS
> - process_event()
> +/**
> + Print the given event, and either delete it or delegate the deletion
> + to someone else.
>
> - RETURN
> - 0 ok and continue
> - 1 error and terminate
> - -1 ok and terminate
> -
> - TODO
> - This function returns 0 even in some error cases. This should be changed.
> + The deletion may be delegated in two cases: (1) the event is a
> + Format_description_log_event, and is saved in
> + glob_description_event; (2) the event is a Create_file_log_event,
> + and is saved in load_processor.
> +
> + @param[in,out] print_event_info Parameters and context state
> + determining how to print.
> + @param[in] ev Log_event to process.
> + @param[in] pos Offset from beginning of binlog file.
> + @param[in] logname Name of input binlog.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> + @retval OK_STOP No error, but the end of the specified range of
> + events to process has been reached and the program should terminate.
> */
> -
> -
> -
> -int process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
> - my_off_t pos)
> +Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
> + my_off_t pos, const char *logname)
> {
> char ll_buff[21];
> Log_event_type ev_type= ev->get_type_code();
> DBUG_ENTER("process_event");
> print_event_info->short_form= short_form;
> + Exit_status retval= OK_CONTINUE;
>
> /*
> Format events are not concerned by --offset and such, we always need to
> @@ -545,14 +675,15 @@ int process_event(PRINT_EVENT_INFO *prin
> start_datetime= 0;
> offset= 0; // print everything and protect against cycling rec_count
> }
> - if (server_id && (server_id != ev->server_id)) {
> - DBUG_RETURN(0);
> - }
> + if (server_id && (server_id != ev->server_id))
> + /* skip just this event, continue processing the log. */
> + goto end;
> if (((my_time_t)(ev->when) >= stop_datetime)
> || (pos >= stop_position_mot))
> {
> - stop_passed= 1; // skip all next binlogs
> - DBUG_RETURN(-1);
> + /* end the program */
> + retval= OK_STOP;
> + goto end;
> }
> if (!short_form)
> fprintf(result_file, "# at %s\n",llstr(pos,ll_buff));
> @@ -568,10 +699,15 @@ int process_event(PRINT_EVENT_INFO *prin
>
> switch (ev_type) {
> case QUERY_EVENT:
> - if (check_database(((Query_log_event*)ev)->db))
> + if (shall_skip_database(((Query_log_event*)ev)->db))
> goto end;
> if (opt_base64_output_mode == BASE64_OUTPUT_ALWAYS)
> - write_event_header_and_base64(ev, result_file, print_event_info);
> + {
> + if ((retval= write_event_header_and_base64(ev, result_file,
> + print_event_info)) !=
> + OK_CONTINUE)
> + goto end;
> + }
> else
> ev->print(result_file, print_event_info);
> break;
> @@ -585,7 +721,7 @@ int process_event(PRINT_EVENT_INFO *prin
> related Append_block and Exec_load.
> Note that Load event from 3.23 is not tested.
> */
> - if (check_database(ce->db))
> + if (shall_skip_database(ce->db))
> goto end; // Next event
> /*
> We print the event, but with a leading '#': this is just to inform
> @@ -596,7 +732,10 @@ int process_event(PRINT_EVENT_INFO *prin
> */
> if (opt_base64_output_mode == BASE64_OUTPUT_ALWAYS)
> {
> - write_event_header_and_base64(ce, result_file, print_event_info);
> + if ((retval= write_event_header_and_base64(ce, result_file,
> + print_event_info)) !=
> + OK_CONTINUE)
> + goto end;
> }
> else
> ce->print(result_file, print_event_info, TRUE);
> @@ -604,17 +743,29 @@ int process_event(PRINT_EVENT_INFO *prin
> // If this binlog is not 3.23 ; why this test??
> if (glob_description_event->binlog_version >= 3)
> {
> - if (load_processor.process(ce))
> - break; // Error
> - ev= 0;
> + /*
> + transfer the responsibility for destroying the event to
> + load_processor
> + */
> + ev= NULL;
> + if ((retval= load_processor.process(ce)) != OK_CONTINUE)
> + goto end;
> }
> break;
> }
> +
> case APPEND_BLOCK_EVENT:
> + /*
> + Append_block_log_events can safely print themselves even if
> + the subsequent call load_processor.process fails, because the
> + output of Append_block_log_event::print is only a comment.
> + */
> ev->print(result_file, print_event_info);
> - if (load_processor.process((Append_block_log_event*) ev))
> - break; // Error
> + if ((retval= load_processor.process((Append_block_log_event*) ev)) !=
> + OK_CONTINUE)
> + goto end;
> break;
> +
> case EXEC_LOAD_EVENT:
> {
> ev->print(result_file, print_event_info);
> @@ -632,8 +783,8 @@ int process_event(PRINT_EVENT_INFO *prin
> delete ce;
> }
> else
> - fprintf(stderr,"Warning: ignoring Exec_load as there is no \
> -Create_file event for file_id: %u\n",exv->file_id);
> + warning("Ignoring Execute_load_log_event as there is no "
> + "Create_file event for file_id: %u", exv->file_id);
> break;
> }
> case FORMAT_DESCRIPTION_EVENT:
> @@ -653,34 +804,34 @@ Create_file event for file_id: %u\n",exv
> if (!force_if_open_opt &&
> (glob_description_event->flags & LOG_EVENT_BINLOG_IN_USE_F))
> {
> - file_not_closed_error= 1;
> - DBUG_RETURN(1);
> + error("Attempting to dump binlog '%s', which was not closed properly. "
> + "Most probably, mysqld is still writing it, or it crashed. "
> + "Rerun with --force-if-open to ignore this problem.", logname);
> + DBUG_RETURN(ERROR_STOP);
> }
> break;
> case BEGIN_LOAD_QUERY_EVENT:
> ev->print(result_file, print_event_info);
> - load_processor.process((Begin_load_query_log_event*) ev);
> + if ((retval= load_processor.process((Begin_load_query_log_event*) ev)) !=
> + OK_CONTINUE)
> + goto end;
> break;
> case EXECUTE_LOAD_QUERY_EVENT:
> {
> Execute_load_query_log_event *exlq= (Execute_load_query_log_event*)ev;
> char *fname= load_processor.grab_fname(exlq->file_id);
>
> - if (check_database(exlq->db))
> + if (!shall_skip_database(exlq->db))
> {
> if (fname)
> - my_free(fname, MYF(MY_WME));
> - goto end;
> + exlq->print(result_file, print_event_info, fname);
> + else
> + warning("Ignoring Execute_load_query since there is no "
> + "Begin_load_query event for file_id: %u", exlq->file_id);
> }
>
> if (fname)
> - {
> - exlq->print(result_file, print_event_info, fname);
> my_free(fname, MYF(MY_WME));
> - }
> - else
> - fprintf(stderr,"Warning: ignoring Execute_load_query as there is no \
> -Begin_load_query event for file_id: %u\n", exlq->file_id);
> break;
> }
> case TABLE_MAP_EVENT:
> @@ -699,20 +850,18 @@ Begin_load_query event for file_id: %u\n
> */
> if (!print_event_info->printed_fd_event && !short_form)
> {
> - /*
> - todo: a lot to clean up here
> - */
> const char* type_str= ev->get_type_str();
> - delete ev;
> if (opt_base64_output_mode == BASE64_OUTPUT_NEVER)
> - die("--base64-output=never specified, but binlog contains a "
> - "%s event which must be printed in base64.",
> - type_str);
> + error("--base64-output=never specified, but binlog contains a "
> + "%s event which must be printed in base64.",
> + type_str);
> else
> - die("malformed binlog: it does not contain any "
> - "Format_description_log_event. I now found a %s event, which is "
> - "not safe to process without a Format_description_log_event.",
> - type_str);
> + error("malformed binlog: it does not contain any "
> + "Format_description_log_event. I now found a %s event, which "
> + "is not safe to process without a "
> + "Format_description_log_event.",
> + type_str);
> + goto err;
> }
> /* FALL THROUGH */
> default:
> @@ -720,6 +869,10 @@ Begin_load_query event for file_id: %u\n
> }
> }
>
> + goto end;
> +
> +err:
> + retval= ERROR_STOP;
> end:
> rec_count++;
> /*
> @@ -732,7 +885,7 @@ end:
> ev->temp_buf= 0;
> delete ev;
> }
> - DBUG_RETURN(0);
> + DBUG_RETURN(retval);
> }
>
>
> @@ -887,16 +1040,71 @@ that may lead to an endless loop.",
> };
>
>
> -void sql_print_error(const char *format,...)
> +/**
> + Auxiliary function used by error() and warning().
> +
> + Prints the given text (normally "WARNING: " or "ERROR: "), followed
> + by the given vprintf-style string, followed by a newline.
> +
> + @param format Printf-style format string.
> + @param args List of arguments for the format string.
> + @param msg Text to print before the string.
> +*/
> +static void error_or_warning(const char *format, va_list args, const char *msg)
> {
> - va_list args;
> - va_start(args, format);
> - fprintf(stderr, "ERROR: ");
> + fprintf(stderr, "%s: ", msg);
> vfprintf(stderr, format, args);
> fprintf(stderr, "\n");
> +}
> +
> +/**
> + Prints a message to stderr, prefixed with the text "ERROR: " and
> + suffixed with a newline.
> +
> + @param format Printf-style format string, followed by printf
> + varargs.
> +*/
> +static void error(const char *format,...)
> +{
> + va_list args;
> + va_start(args, format);
> + error_or_warning(format, args, "ERROR");
> va_end(args);
> }
>
> +
> +/**
> + This function is used in log_event.cc to report errors.
> +
> + @param format Printf-style format string, followed by printf
> + varargs.
> +*/
> +static void sql_print_error(const char *format,...)
> +{
> + va_list args;
> + va_start(args, format);
> + error_or_warning(format, args, "ERROR");
> + va_end(args);
> +}
> +
> +/**
> + Prints a message to stderr, prefixed with the text "WARNING: " and
> + suffixed with a newline.
> +
> + @param format Printf-style format string, followed by printf
> + varargs.
> +*/
> +static void warning(const char *format,...)
> +{
> + va_list args;
> + va_start(args, format);
> + error_or_warning(format, args, "WARNING");
> + va_end(args);
> +}
> +
> +/**
> + Frees memory for global variables in this file.
> +*/
> static void cleanup()
> {
> my_free(pass,MYF(MY_ALLOW_ZERO_PTR));
> @@ -904,20 +1112,10 @@ static void cleanup()
> my_free((char*) host, MYF(MY_ALLOW_ZERO_PTR));
> my_free((char*) user, MYF(MY_ALLOW_ZERO_PTR));
> my_free((char*) dirname_for_local_load, MYF(MY_ALLOW_ZERO_PTR));
> -}
>
> -static void die(const char* fmt, ...)
> -{
> - va_list args;
> - va_start(args, fmt);
> - fprintf(stderr, "ERROR: ");
> - vfprintf(stderr, fmt, args);
> - fprintf(stderr, "\n");
> - va_end(args);
> - cleanup();
> - /* We cannot free DBUG, it is used in global destructors after exit(). */
> - my_end(my_end_arg | MY_DONT_FREE_DBUG);
> - exit(1);
> + delete glob_description_event;
> + if (mysql)
> + mysql_close(mysql);
> }
>
> #include <help_start.h>
> @@ -955,7 +1153,7 @@ static my_time_t convert_str_to_timestam
> if (str_to_datetime(str, strlen(str), &l_time, 0, &was_cut) !=
> MYSQL_TIMESTAMP_DATETIME || was_cut)
> {
> - fprintf(stderr, "Incorrect date and time argument: %s\n", str);
> + error("Incorrect date and time argument: %s", str);
> exit(1);
> }
> /*
> @@ -1056,34 +1254,56 @@ static int parse_args(int *argc, char***
> return 0;
> }
>
> -static MYSQL* safe_connect()
> +
> +/**
> + Create and initialize the global mysql object, and connect to the
> + server.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +static Exit_status safe_connect()
> {
> - MYSQL *local_mysql= mysql_init(NULL);
> + mysql= mysql_init(NULL);
>
> - if (!local_mysql)
> - die("Failed on mysql_init");
> + if (!mysql)
> + {
> + error("Failed on mysql_init.");
> + return ERROR_STOP;
> + }
>
> if (opt_protocol)
> - mysql_options(local_mysql, MYSQL_OPT_PROTOCOL, (char*) &opt_protocol);
> - if (!mysql_real_connect(local_mysql, host, user, pass, 0, port, sock, 0))
> + mysql_options(mysql, MYSQL_OPT_PROTOCOL, (char*) &opt_protocol);
> + if (!mysql_real_connect(mysql, host, user, pass, 0, port, sock, 0))
> {
> - char errmsg[256];
> - strmake(errmsg, mysql_error(local_mysql), sizeof(errmsg)-1);
> - mysql_close(local_mysql);
> - die("failed on connect: %s", errmsg);
> + error("Failed on connect: %s", mysql_error(mysql));
> + return ERROR_STOP;
> }
> - local_mysql->reconnect= 1;
> - return local_mysql;
> + mysql->reconnect= 1;
> + return OK_CONTINUE;
> }
>
>
> -static int dump_log_entries(const char* logname)
> +/**
> + High-level function for dumping a named binlog.
> +
> + This function calls dump_remote_log_entries() or
> + dump_local_log_entries() to do the job.
> +
> + @param[in] logname Name of input binlog.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> + @retval OK_STOP No error, but the end of the specified range of
> + events to process has been reached and the program should terminate.
> +*/
> +static Exit_status dump_log_entries(const char* logname)
> {
> - int rc;
> + Exit_status rc;
> PRINT_EVENT_INFO print_event_info;
>
> if (!print_event_info.init_ok())
> - return 1;
> + return ERROR_STOP;
> /*
> Set safe delimiter, to dump things
> like CREATE PROCEDURE safely
> @@ -1101,51 +1321,50 @@ static int dump_log_entries(const char*
> }
>
>
> -/*
> - This is not as smart as check_header() (used for local log); it will not work
> - for a binlog which mixes format. TODO: fix this.
> -*/
> -static int check_master_version(MYSQL *mysql_arg,
> - Format_description_log_event
> - **description_event)
> +/**
> + When reading a remote binlog, this function is used to grab the
> + Format_description_log_event in the beginning of the stream.
> +
> + This is not as smart as check_header() (used for local log); it will
> + not work for a binlog which mixes format. TODO: fix this.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> +*/
> +static Exit_status check_master_version()
> {
> MYSQL_RES* res = 0;
> MYSQL_ROW row;
> const char* version;
>
> - if (mysql_query(mysql_arg, "SELECT VERSION()") ||
> - !(res = mysql_store_result(mysql_arg)))
> + if (mysql_query(mysql, "SELECT VERSION()") ||
> + !(res = mysql_store_result(mysql)))
> {
> - /* purecov: begin inspected */
> - char errmsg[256];
> - strmake(errmsg, mysql_error(mysql_arg), sizeof(errmsg)-1);
> - mysql_close(mysql_arg);
> - die("Error checking master version: %s", errmsg);
> - /* purecov: end */
> + error("Could not find server version: "
> + "Query failed when checking master version: %s", mysql_error(mysql));
> + return ERROR_STOP;
> }
> if (!(row = mysql_fetch_row(res)))
> {
> - /* purecov: begin inspected */
> - mysql_free_result(res);
> - mysql_close(mysql);
> - die("Master returned no rows for SELECT VERSION()");
> - /* purecov: end */
> + error("Could not find server version: "
> + "Master returned no rows for SELECT VERSION().");
> + goto err;
> }
> +
> if (!(version = row[0]))
> {
> - /* purecov: begin inspected */
> - mysql_free_result(res);
> - mysql_close(mysql_arg);
> - die("Master reported NULL for the version");
> - /* purecov: end */
> + error("Could not find server version: "
> + "Master reported NULL for the version.");
> + goto err;
> }
>
> + delete glob_description_event;
> switch (*version) {
> case '3':
> - *description_event= new Format_description_log_event(1);
> + glob_description_event= new Format_description_log_event(1);
> break;
> case '4':
> - *description_event= new Format_description_log_event(3);
> + glob_description_event= new Format_description_log_event(3);
> break;
> case '5':
> /*
> @@ -1154,31 +1373,53 @@ static int check_master_version(MYSQL *m
> So we first assume that this is 4.0 (which is enough to read the
> Format_desc event if one comes).
> */
> - *description_event= new Format_description_log_event(3);
> + glob_description_event= new Format_description_log_event(3);
> break;
> default:
> - /* purecov: begin inspected */
> - mysql_free_result(res);
> - mysql_close(mysql_arg);
> - die("Master reported unrecognized MySQL version '%s'", version);
> - /* purecov: end */
> + glob_description_event= NULL;
> + error("Could not find server version: "
> + "Master reported unrecognized MySQL version '%s'.", version);
> + goto err;
> + }
> + if (!glob_description_event || !glob_description_event->is_valid())
> + {
> + error("Failed creating Format_description_log_event; out of memory?");
> + goto err;
> }
> +
> mysql_free_result(res);
> - return 0;
> + return OK_CONTINUE;
> +
> +err:
> + mysql_free_result(res);
> + return ERROR_STOP;
> }
>
>
> -static int dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
> - const char* logname)
> +/**
> + Requests binlog dump from a remote server and prints the events it
> + receives.
> +
> + @param[in,out] print_event_info Parameters and context state
> + determining how to print.
> + @param[in] logname Name of input binlog.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> + @retval OK_STOP No error, but the end of the specified range of
> + events to process has been reached and the program should terminate.
> +*/
> +static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
> + const char* logname)
>
> {
> uchar buf[128];
> ulong len;
> uint logname_len;
> NET* net;
> - int error= 0;
> my_off_t old_off= start_position_mot;
> char fname[FN_REFLEN+1];
> + Exit_status retval= OK_CONTINUE;
> DBUG_ENTER("dump_remote_log_entries");
>
> /*
> @@ -1186,20 +1427,12 @@ static int dump_remote_log_entries(PRINT
> we cannot re-use the same connection as before, because it is now dead
> (COM_BINLOG_DUMP kills the thread when it finishes).
> */
> - mysql= safe_connect();
> + if ((retval= safe_connect()) != OK_CONTINUE)
> + DBUG_RETURN(retval);
> net= &mysql->net;
>
> - if (check_master_version(mysql, &glob_description_event))
> - {
> - fprintf(stderr, "Could not find server version");
> - DBUG_RETURN(1);
> - }
> - if (!glob_description_event || !glob_description_event->is_valid())
> - {
> - fprintf(stderr, "Invalid Format_description log event; \
> -could be out of memory");
> - DBUG_RETURN(1);
> - }
> + if ((retval= check_master_version()) != OK_CONTINUE)
> + DBUG_RETURN(retval);
>
> /*
> COM_BINLOG_DUMP accepts only 4 bytes for the position, so we are forced to
> @@ -1211,18 +1444,16 @@ could be out of memory");
> size_t tlen = strlen(logname);
> if (tlen > UINT_MAX)
> {
> - fprintf(stderr,"Log name too long\n");
> - error= 1;
> - goto err;
> + error("Log name too long.");
> + DBUG_RETURN(ERROR_STOP);
> }
> logname_len = (uint) tlen;
> int4store(buf + 6, 0);
> memcpy(buf + 10, logname, logname_len);
> if (simple_command(mysql, COM_BINLOG_DUMP, buf, logname_len + 10, 1))
> {
> - fprintf(stderr,"Got fatal error sending the log dump command\n");
> - error= 1;
> - goto err;
> + error("Got fatal error sending the log dump command.");
> + DBUG_RETURN(ERROR_STOP);
> }
>
> for (;;)
> @@ -1233,10 +1464,8 @@ could be out of memory");
> len= cli_safe_read(mysql);
> if (len == packet_error)
> {
> - fprintf(stderr, "Got error reading packet from server: %s\n",
> - mysql_error(mysql));
> - error= 1;
> - goto err;
> + error("Got error reading packet from server: %s", mysql_error(mysql));
> + DBUG_RETURN(ERROR_STOP);
> }
> if (len < 8 && net->read_pos[0] == 254)
> break; // end of data
> @@ -1246,9 +1475,8 @@ could be out of memory");
> len - 1, &error_msg,
> glob_description_event)))
> {
> - fprintf(stderr, "Could not construct log event object\n");
> - error= 1;
> - goto err;
> + error("Could not construct log event object: %s", error_msg);
> + DBUG_RETURN(ERROR_STOP);
> }
> /*
> If reading from a remote host, ensure the temp_buf for the
> @@ -1287,8 +1515,7 @@ could be out of memory");
> if ((rev->ident_len != logname_len) ||
> memcmp(rev->new_log_ident, logname, logname_len))
> {
> - error= 0;
> - goto err;
> + DBUG_RETURN(OK_CONTINUE);
> }
> /*
> Otherwise, this is a fake Rotate for our log, at the very
> @@ -1313,11 +1540,9 @@ could be out of memory");
> if (old_off != BIN_LOG_HEADER_SIZE)
> len= 1; // fake event, don't increment old_off
> }
> - if ((error= process_event(print_event_info, ev, old_off)))
> - {
> - error= ((error < 0) ? 0 : 1);
> - goto err;
> - }
> + Exit_status retval= process_event(print_event_info, ev, old_off, logname);
> + if (retval != OK_CONTINUE)
> + DBUG_RETURN(retval);
> }
> else
> {
> @@ -1325,26 +1550,21 @@ could be out of memory");
> const char *old_fname= le->fname;
> uint old_len= le->fname_len;
> File file;
> + Exit_status retval;
>
> if ((file= load_processor.prepare_new_file_for_old_format(le,fname)) < 0)
> - {
> - error= 1;
> - goto err;
> - }
> + DBUG_RETURN(ERROR_STOP);
>
> - if ((error= process_event(print_event_info, ev, old_off)))
> + retval= process_event(print_event_info, ev, old_off, logname);
> + if (retval != OK_CONTINUE)
> {
> my_close(file,MYF(MY_WME));
> - error= ((error < 0) ? 0 : 1);
> - goto err;
> + DBUG_RETURN(retval);
> }
> - error= load_processor.load_old_format_file(net,old_fname,old_len,file);
> + retval= load_processor.load_old_format_file(net,old_fname,old_len,file);
> my_close(file,MYF(MY_WME));
> - if (error)
> - {
> - error= 1;
> - goto err;
> - }
> + if (retval != OK_CONTINUE)
> + DBUG_RETURN(retval);
> }
> /*
> Let's adjust offset for remote log as for local log to produce
> @@ -1353,15 +1573,13 @@ could be out of memory");
> old_off+= len-1;
> }
>
> -err:
> - mysql_close(mysql);
> - DBUG_RETURN(error);
> + DBUG_RETURN(OK_CONTINUE);
> }
>
>
> /**
> - Reads the @c Format_description_log_event from the beginning of the
> - input file.
> + Reads the @c Format_description_log_event from the beginning of a
> + local input file.
>
> The @c Format_description_log_event is only read if it is outside
> the range specified with @c --start-position; otherwise, it will be
> @@ -1375,32 +1593,42 @@ err:
> @param file The file to which a @c Format_description_log_event will
> be printed.
>
> - @param description_event Pointer to the global @c
> - Format_description_log_event pointer. This will be updated if a new
> - Format_description_log_event is found.
> -
> - @param print_event_info Context state needed to print events.
> -*/
> -static void check_header(IO_CACHE* file,
> - Format_description_log_event **description_event,
> - PRINT_EVENT_INFO *print_event_info)
> + @param[in,out] print_event_info Parameters and context state
> + determining how to print.
> +
> + @param[in] logname Name of input binlog.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> + @retval OK_STOP No error, but the end of the specified range of
> + events to process has been reached and the program should terminate.
> +*/
> +static Exit_status check_header(IO_CACHE* file,
> + PRINT_EVENT_INFO *print_event_info,
> + const char* logname)
> {
> uchar header[BIN_LOG_HEADER_SIZE];
> uchar buf[PROBE_HEADER_LEN];
> my_off_t tmp_pos, pos;
>
> - *description_event= new Format_description_log_event(3);
> + delete glob_description_event;
> + if (!(glob_description_event= new Format_description_log_event(3)))
> + {
> + error("Failed creating Format_description_log_event; out of memory?");
> + return ERROR_STOP;
> + }
> +
> pos= my_b_tell(file);
> my_b_seek(file, (my_off_t)0);
> if (my_b_read(file, header, sizeof(header)))
> {
> - delete *description_event;
> - die("Failed reading header; Probably an empty file");
> + error("Failed reading header; probably an empty file.");
> + return ERROR_STOP;
> }
> if (memcmp(header, BINLOG_MAGIC, sizeof(header)))
> {
> - delete *description_event;
> - die("File is not a binary log file");
> + error("File is not a binary log file.");
> + return ERROR_STOP;
> }
>
> /*
> @@ -1423,10 +1651,9 @@ static void check_header(IO_CACHE* file,
> {
> if (file->error)
> {
> - delete *description_event;
> - die("\
> -Could not read entry at offset %lu : Error in log format or read error",
> - tmp_pos);
> + error("Could not read entry at offset %llu: "
> + "Error in log format or read error.", (ulonglong)tmp_pos);
> + return ERROR_STOP;
> }
> /*
> Otherwise this is just EOF : this log currently contains 0-2
> @@ -1456,8 +1683,13 @@ Could not read entry at offset %lu : Err
> (LOG_EVENT_MINIMAL_HEADER_LEN + START_V3_HEADER_LEN))
> {
> /* This is 3.23 (format 1) */
> - delete *description_event;
> - *description_event= new Format_description_log_event(1);
> + delete glob_description_event;
> + if (!(glob_description_event= new Format_description_log_event(1)))
> + {
> + error("Failed creating Format_description_log_event; "
> + "out of memory?");
> + return ERROR_STOP;
> + }
> }
> break;
> }
> @@ -1469,26 +1701,32 @@ Could not read entry at offset %lu : Err
> Format_description_log_event *new_description_event;
> my_b_seek(file, tmp_pos); /* seek back to event's start */
> if (!(new_description_event= (Format_description_log_event*)
> - Log_event::read_log_event(file, *description_event)))
> + Log_event::read_log_event(file, glob_description_event)))
> /* EOF can't be hit here normally, so it's a real error */
> {
> - delete *description_event;
> - die("Could not read a Format_description_log_event event \
> -at offset %lu ; this could be a log format error or read error",
> - tmp_pos);
> + error("Could not read a Format_description_log_event event at "
> + "offset %llu; this could be a log format error or read error.",
> + (ulonglong)tmp_pos);
> + return ERROR_STOP;
> }
> if (opt_base64_output_mode == BASE64_OUTPUT_AUTO
> || opt_base64_output_mode == BASE64_OUTPUT_ALWAYS)
> + {
> /*
> process_event will delete *description_event and set it to
> the new one, so we should not do it ourselves in this
> case.
> */
> - process_event(print_event_info, new_description_event, tmp_pos);
> + Exit_status retval= process_event(print_event_info,
> + new_description_event, tmp_pos,
> + logname);
> + if (retval != OK_CONTINUE)
> + return retval;
> + }
> else
> {
> - delete *description_event;
> - *description_event= new_description_event;
> + delete glob_description_event;
> + glob_description_event= new_description_event;
> }
> DBUG_PRINT("info",("Setting description_event"));
> }
> @@ -1496,12 +1734,13 @@ at offset %lu ; this could be a log form
> {
> Log_event *ev;
> my_b_seek(file, tmp_pos); /* seek back to event's start */
> - if (!(ev= Log_event::read_log_event(file, *description_event)))
> - /* EOF can't be hit here normally, so it's a real error */
> + if (!(ev= Log_event::read_log_event(file, glob_description_event)))
> {
> - delete *description_event;
> - die("Could not read a Rotate_log_event event at offset %lu ;"
> - " this could be a log format error or read error", tmp_pos);
> + /* EOF can't be hit here normally, so it's a real error */
> + error("Could not read a Rotate_log_event event at offset %llu;"
> + " this could be a log format error or read error.",
> + (ulonglong)tmp_pos);
> + return ERROR_STOP;
> }
> delete ev;
> }
> @@ -1510,31 +1749,48 @@ at offset %lu ; this could be a log form
> }
> }
> my_b_seek(file, pos);
> + return OK_CONTINUE;
> }
>
>
> -static int dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
> - const char* logname)
> +/**
> + Reads a local binlog and prints the events it sees.
> +
> + @param[in] logname Name of input binlog.
> +
> + @param[in,out] print_event_info Parameters and context state
> + determining how to print.
> +
> + @retval ERROR_STOP An error occurred - the program should terminate.
> + @retval OK_CONTINUE No error, the program should continue.
> + @retval OK_STOP No error, but the end of the specified range of
> + events to process has been reached and the program should terminate.
> +*/
> +static Exit_status dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
> + const char* logname)
> {
> File fd = -1;
> IO_CACHE cache,*file= &cache;
> uchar tmp_buff[BIN_LOG_HEADER_SIZE];
> - int error= 0;
> + Exit_status retval= OK_CONTINUE;
>
> if (logname && strcmp(logname, "-") != 0)
> {
> + /* read from normal file */
> if ((fd = my_open(logname, O_RDONLY | O_BINARY, MYF(MY_WME))) < 0)
> - return 1;
> + return ERROR_STOP;
> if (init_io_cache(file, fd, 0, READ_CACHE, start_position_mot, 0,
> MYF(MY_WME | MY_NABP)))
> {
> my_close(fd, MYF(MY_WME));
> - return 1;
> + return ERROR_STOP;
> }
> - check_header(file, &glob_description_event, print_event_info);
> + if ((retval= check_header(file, print_event_info, logname)) != OK_CONTINUE)
> + goto end;
> }
> - else // reading from stdin;
> + else
> {
> + /* read from stdin */
> /*
> Windows opens stdin in text mode by default. Certain characters
> such as CTRL-Z are interpeted as events and the read() method
> @@ -1546,14 +1802,18 @@ static int dump_local_log_entries(PRINT_
> #if defined (__WIN__) || (_WIN64)
> if (_setmode(fileno(stdin), O_BINARY) == -1)
> {
> - fprintf(stderr, "Could not set binary mode on stdin.\n");
> - return 1;
> + error("Could not set binary mode on stdin.");
> + return ERROR_STOP;
> }
> #endif
> if (init_io_cache(file, fileno(stdin), 0, READ_CACHE, (my_off_t) 0,
> 0, MYF(MY_WME | MY_NABP | MY_DONT_CHECK_FILESIZE)))
> - return 1;
> - check_header(file, &glob_description_event, print_event_info);
> + {
> + error("Failed to init IO cache.");
> + return ERROR_STOP;
> + }
> + if ((retval= check_header(file, print_event_info, logname)) != OK_CONTINUE)
> + goto end;
> if (start_position)
> {
> /* skip 'start_position' characters from stdin */
> @@ -1564,8 +1824,8 @@ static int dump_local_log_entries(PRINT_
> tmp=min(length,sizeof(buff));
> if (my_b_read(file, buff, (uint) tmp))
> {
> - error= 1;
> - goto end;
> + error("Failed reading from file.");
> + goto err;
> }
> }
> }
> @@ -1573,14 +1833,14 @@ static int dump_local_log_entries(PRINT_
>
> if (!glob_description_event || !glob_description_event->is_valid())
> {
> - delete glob_description_event;
> - die("Invalid Format_description log event; could be out of memory");
> + error("Invalid Format_description log event; could be out of memory.");
> + goto err;
> }
>
> if (!start_position && my_b_read(file, tmp_buff, BIN_LOG_HEADER_SIZE))
> {
> - error= 1;
> - goto end;
> + error("Failed reading from file.");
> + goto err;
> }
> for (;;)
> {
> @@ -1598,36 +1858,36 @@ static int dump_local_log_entries(PRINT_
> file->error= 0;
> else if (file->error)
> {
> - fprintf(stderr,
> - "Could not read entry at offset %s:"
> - "Error in log format or read error\n",
> - llstr(old_off,llbuff));
> - error= 1;
> + error("Could not read entry at offset %s: "
> + "Error in log format or read error.",
> + llstr(old_off,llbuff));
> + goto err;
> }
> // file->error == 0 means EOF, that's OK, we break in this case
> - break;
> - }
> - if ((error= process_event(print_event_info, ev, old_off)))
> - {
> - if (error < 0)
> - error= 0;
> - break;
> + goto end;
> }
> + if ((retval= process_event(print_event_info, ev, old_off, logname)) !=
> + OK_CONTINUE)
> + goto end;
> }
>
> + /* NOTREACHED */
> +
> +err:
> + retval= ERROR_STOP;
> +
> end:
> if (fd >= 0)
> my_close(fd, MYF(MY_WME));
> end_io_cache(file);
> - delete glob_description_event;
> - return error;
> + return retval;
> }
>
>
> int main(int argc, char** argv)
> {
> char **defaults_argv;
> - int exit_value= 0;
> + Exit_status retval= OK_CONTINUE;
> ulonglong save_stop_position;
> MY_INIT(argv[0]);
> DBUG_ENTER("main");
> @@ -1689,15 +1949,13 @@ int main(int argc, char** argv)
> "\n/*!40101 SET NAMES %s */;\n", charset);
>
> for (save_stop_position= stop_position, stop_position= ~(my_off_t)0 ;
> - (--argc >= 0) && !stop_passed ; )
> + (--argc >= 0) ; )
> {
> if (argc == 0) // last log, --stop-position applies
> stop_position= save_stop_position;
> - if (dump_log_entries(*(argv++)))
> - {
> - exit_value=1;
> + if ((retval= dump_log_entries(*argv++)) != OK_CONTINUE)
> break;
> - }
> +
> // For next log, --start-position does not apply
> start_position= BIN_LOG_HEADER_SIZE;
> }
> @@ -1729,17 +1987,9 @@ int main(int argc, char** argv)
> /* We cannot free DBUG, it is used in global destructors after exit(). */
> my_end(my_end_arg | MY_DONT_FREE_DBUG);
>
> - if (file_not_closed_error)
> - {
> - fprintf(stderr,
> -"\nError: attempting to dump binlog '%s' which was not closed properly.\n"
> -"Most probably mysqld is still writting it, or crashed.\n"
> -"Your current options specify --disable-force-if-open\n"
> -"which means to abort on this problem.\n"
> -"You can rerun using --force-if-open to ignore this problem.\n\n", argv[-1]);
> - }
> - exit(exit_value);
> - DBUG_RETURN(exit_value); // Keep compilers happy
> + exit(retval == ERROR_STOP ? 1 : 0);
> + /* Keep compilers happy. */
> + DBUG_RETURN(retval == ERROR_STOP ? 1 : 0);
> }
>
> /*
> diff -Nrup a/mysql-test/suite/binlog/r/binlog_base64_flag.result
> b/mysql-test/suite/binlog/r/binlog_base64_flag.result
> --- a/mysql-test/suite/binlog/r/binlog_base64_flag.result 2007-12-14 19:01:59 +01:00
> +++ b/mysql-test/suite/binlog/r/binlog_base64_flag.result 2008-02-05 16:55:41 +01:00
> @@ -40,8 +40,13 @@ SET @@session.foreign_key_checks=1, @@se
> SET @@session.sql_mode=0/*!*/;
> /*!\C latin1 *//*!*/;
> SET
> @@session.character_set_client=8,@@session.collation_connection=8,@@session.collation_server=8/*!*/;
> -create table t1 (a int) engine= myisam/*!*/;
> +create table t1 (a int) engine= myisam
> +/*!*/;
> # at 203
> +DELIMITER ;
> +# End of log file
> +ROLLBACK /* added by mysqlbinlog */;
> +/*!50003 SET COMPLETION_TYPE=@OLD_COMPLETION_TYPE*/;
> ==== Test non-matching FD event and Row event ====
> BINLOG '
> 4CdYRw8BAAAAYgAAAGYAAAAAAAQANS4xLjE1LW5kYi02LjEuMjQtZGVidWctbG9nAAAAAAAAAAAA
> diff -Nrup a/mysql-test/suite/binlog/t/binlog_killed.test
> b/mysql-test/suite/binlog/t/binlog_killed.test
> --- a/mysql-test/suite/binlog/t/binlog_killed.test 2007-11-06 19:35:12 +01:00
> +++ b/mysql-test/suite/binlog/t/binlog_killed.test 2008-02-05 16:55:41 +01:00
> @@ -39,7 +39,7 @@ connection con2;
> reap;
> let $rows= `select count(*) from t2 /* must be 2 or 0 */`;
>
> ---exec $MYSQL_BINLOG --start-position=134 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/kill_query_calling_sp.binlog
> +--exec $MYSQL_BINLOG --force-if-open --start-position=134
> $MYSQLTEST_VARDIR/log/master-bin.000001 >
> $MYSQLTEST_VARDIR/tmp/kill_query_calling_sp.binlog
> --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> eval select
> (@a:=load_file("$MYSQLTEST_VARDIR/tmp/kill_query_calling_sp.binlog"))
> @@ -250,7 +250,7 @@ source include/show_binlog_events.inc;
>
> # a proof the query is binlogged with an error
>
> ---exec $MYSQL_BINLOG --start-position=106 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> +--exec $MYSQL_BINLOG --force-if-open --start-position=106
> $MYSQLTEST_VARDIR/log/master-bin.000001 >
> $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> eval select
> (@a:=load_file("$MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog"))
> @@ -296,7 +296,7 @@ source include/show_binlog_events.inc;
>
> # a proof the query is binlogged with an error
>
> ---exec $MYSQL_BINLOG --start-position=106 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> +--exec $MYSQL_BINLOG --force-if-open --start-position=106
> $MYSQLTEST_VARDIR/log/master-bin.000001 >
> $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> eval select
> (@a:=load_file("$MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog"))
> diff -Nrup a/mysql-test/suite/binlog/t/binlog_killed_simulate.test
> b/mysql-test/suite/binlog/t/binlog_killed_simulate.test
> --- a/mysql-test/suite/binlog/t/binlog_killed_simulate.test 2007-11-06 13:00:52
> +01:00
> +++ b/mysql-test/suite/binlog/t/binlog_killed_simulate.test 2008-02-05 16:55:41
> +01:00
> @@ -23,7 +23,7 @@ update t1 set a=2 /* will be "killed" af
> #todo: introduce a suite private macro that provides numeric values
> # for some constants like the offset of the first real event
> # that is different between severs versions.
> ---exec $MYSQL_BINLOG --start-position=106 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> +--exec $MYSQL_BINLOG --force-if-open --start-position=106
> $MYSQLTEST_VARDIR/log/master-bin.000001 >
> $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> eval select
> (@a:=load_file("$MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog"))
> @@ -51,7 +51,7 @@ load data infile '../std_data_ln/rpl_loa
>
> source include/show_binlog_events.inc;
>
> ---exec $MYSQL_BINLOG --start-position=98 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> +--exec $MYSQL_BINLOG --force-if-open --start-position=98
> $MYSQLTEST_VARDIR/log/master-bin.000001 >
> $MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog
> --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> eval select
> (@a:=load_file("$MYSQLTEST_VARDIR/tmp/binlog_killed_bug27571.binlog"))
> diff -Nrup a/mysql-test/suite/binlog/t/disabled.def
> b/mysql-test/suite/binlog/t/disabled.def
> --- a/mysql-test/suite/binlog/t/disabled.def 2007-12-14 19:01:59 +01:00
> +++ b/mysql-test/suite/binlog/t/disabled.def 2008-02-05 16:55:41 +01:00
> @@ -9,4 +9,3 @@
> # Do not use any TAB characters for whitespace.
> #
> ##############################################################################
> -binlog_base64_flag : BUG#33247 2007-12-14 Sven: mysqlbinlog does not clean up after
> itself on termination. When compiled in debug mode, this test generates lots of warnings
> for memory leaks.
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>