Hi Mats!
Thanks, I've fixed these issues and re-committed.
I kept the @todo, but added clarification per our IRC discussion.
I also replaced a hard-coded 1 by the symbolic name MY_REPLACE_DIR in
the call to fn_format from
Load_log_processor::prepare_new_file_for_old_format.
/Sven
Mats Kindahl wrote:
> Hi Sven!
>
> In general, the patch is good, but I would like to see better Doxygen
> comments:
>
> * Give a description for all parameters to functions (where you have
> Doxygen comments)
> * Enumerate all return values using @retval
>
> Note that the Doxygen comments will be read by people out of context, so
> repeating documentation (e.g., the return values even when they are
> similar between functions) to avoid anybody having to flip back and
> forth improves the documentation and makes it easier to read.
>
> Just my few cents,
> Mats Kindahl
>
> Sven Sandberg wrote:
>> 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-01-31 16:18:51+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-01-31 16:18:48+01:00,
>> sven@riska.(none) +529 -388
>> - 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-01-31
>> 16:18:48+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-01-31
>> 16:18:48+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-01-31 16:18:48+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-01-31
>> 16:18:48+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-01-31 16:18:48 +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,27 @@ 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, this will contain afiles many elements as
>> the
>> + number of files loaded since the server started. /Sven
>> + */
>> DYNAMIC_ARRAY file_names;
>>
>
> The number of files in the array is no argument for making it a hash.
> Arguments for making it a hash could be:
>
> - It contains many files *and* it is searched frequently,
> - It is easier to use a hash for pragmatic reasons
>
> Neither of which seems to be true.
>
>>
>> - /*
>> - 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 +216,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.
>> +
>> + @param file_id File id identifying LOAD DATA statement.
>>
>> - 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.
>> + @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 +244,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 file_id - file_id identifiying 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 +274,22 @@ 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);
>> };
>>
>>
>> -
>> +/**
>> + @return File handle >= 0 on success, -1 on error.
>> +*/
>>
>
> Please fill in a more elaborate description of what the function does.
>
>> File
>> Load_log_processor::prepare_new_file_for_old_format(Load_log_event *le,
>> char *filename)
>> {
>> @@ -290,7 +303,7 @@ File Load_log_processor::prepare_new_fil
>> 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 +313,21 @@ 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)
>> +/**
>> + @return Exit_status indicating to the caller how to proceed.
>>
>
> Use @retval to enumerate the possible return values.
>
>> +*/
>> +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 +337,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 +349,62 @@ 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 LOAD DATA statement
>> + @param ce pointer to Create_file event object if we are processing
>> + this type of event.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>>
>
> Please use @retval to enumerate possible return values.
>
>> +*/
>> +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 +414,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 +425,34 @@ 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*)
>> + @return Exit_status indicating to the caller how to proceed.
>> +*/
>> +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 +462,26 @@ 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*)
>> + @return Exit_status indicating to the caller how to proceed.
>>
>
> Please use @retval to enumerate the exit status and what they mean.
>
>> +*/
>> +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 file contents to the file.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>>
>
> See above.
>
>> +*/
>> +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 +491,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 +516,23 @@ 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(ERROR_STOP);
>>
>
> You print a warning but the program should stop?
>
>> }
>>
>>
>> -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.
>>
>> -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 +540,17 @@ static bool check_database(const char *l
>> }
>>
>>
>> +/**
>> + Prints the given event in base64 format.
>> +
>> + 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.
>>
>> -static int
>> + @return Exit_status indicating to the caller how to proceed.
>>
>
> See above.
>
>> +*/
>> +static Exit_status
>> write_event_header_and_base64(Log_event *ev, FILE *result_file,
>> PRINT_EVENT_INFO *print_event_info)
>> {
>> @@ -497,35 +563,35 @@ 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
>> +/**
>> + Print the given event, and either delete it or delegate the deletion
>> + to someone else.
>>
>> - SYNOPSIS
>> - process_event()
>> + 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.
>>
>> - 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.
>> + @return Exit_status indicating to the caller how to proceed.
>> */
>> -
>> -
>> -
>> -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 +611,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 +635,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 +657,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 +668,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 +679,24 @@ 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:
>> 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 +714,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 +735,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 +781,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 +800,10 @@ Begin_load_query event for file_id: %u\n
>> }
>> }
>>
>> + goto end;
>> +
>> +err:
>> + retval= ERROR_STOP;
>> end:
>> rec_count++;
>> /*
>> @@ -732,7 +816,7 @@ end:
>> ev->temp_buf= 0;
>> delete ev;
>> }
>> - DBUG_RETURN(0);
>> + DBUG_RETURN(retval);
>> }
>>
>>
>> @@ -887,16 +971,55 @@ that may lead to an endless loop.",
>> };
>>
>>
>> -void sql_print_error(const char *format,...)
>> +/**
>> + Auxiliary function used by error() and warning().
>> +*/
>>
>
> Please add parameters to the Doxygen description.
>
>> +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 the given printf-formatted parameters to stderr, prefixed
>> + with the text "ERROR: " and suffixed with a newline.
>>
>
> Same here.
>
>> +*/
>> +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.
>> +*/
>>
>
> ... and here.
>
>> +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 the given printf-formatted parameters to stderr, prefixed
>> + with the text "WARNING: " and suffixed with a newline.
>> +*/
>>
>
> ... and here.
>
>> +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 +1027,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 +1068,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 +1169,50 @@ 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.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>> +*/
>> +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.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>> +*/
>> +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 +1230,49 @@ static int dump_log_entries(const char* }
>>
>>
>> -/*
>> +/**
>> + 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.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>> */
>> -static int check_master_version(MYSQL *mysql_arg,
>> - Format_description_log_event
>> - **description_event)
>> +static Exit_status check_master_version(void)
>>
>
> This is not C: skip the "void" inside the parentheses.
>
>> {
>> 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 +1281,46 @@ 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.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>> +*/
>>
>
> Parameter description?
>
>> +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 +1328,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 +1345,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 +1365,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 +1376,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 +1416,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 +1441,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 +1451,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 +1474,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
>> @@ -1380,27 +1499,35 @@ err:
>> Format_description_log_event is found.
>>
>> @param print_event_info Context state needed to print events.
>> +
>> + @return Exit_status indicating to the caller how to proceed.
>> */
>> -static void check_header(IO_CACHE* file, -
>> Format_description_log_event **description_event,
>> - PRINT_EVENT_INFO *print_event_info)
>> +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 +1550,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 +1582,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 +1600,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 +1633,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 +1648,40 @@ 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.
>> + + @return Exit_status indicating to the caller how to proceed.
>> +*/
>> +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 +1693,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 +1715,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 +1724,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 +1749,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 +1840,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 +1878,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-01-31 16:18:48 +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-01-31
>> 16:18:48 +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-01-31 16:18:48 +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-01-31 16:18:48
>> +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.
>>
>>
>
>
--
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com
