Below is the list of changes that have just been committed into a local
5.0 repository of guilhem. When guilhem 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
1.1935 05/09/06 14:56:57 guilhem@stripped +7 -0
WL#1012: continuing to implement my review comments:
catching return codes of all memory allocation, of bitmap_init() etc (some other functions
still have to be caught, like open_tables()).
Plus comments and whitespace change to minimize diff against the main 5.0.
sql/sql_class.h
1.258 05/09/06 14:56:53 guilhem@stripped +2 -1
comment fix.
sql/sql_class.cc
1.203 05/09/06 14:56:53 guilhem@stripped +22 -30
testing memory allocation failures, add_row_data() failures.
s/memory/row_data to unify names.
sql/log_event.h
1.121 05/09/06 14:56:53 guilhem@stripped +17 -13
more specialized is_valid() for new row-based binlog events, to detect allocation errors.
Adding "const" to a method to make it so much more beautiful. Indentation.
sql/log_event.cc
1.196 05/09/06 14:56:53 guilhem@stripped +27 -21
adding MY_WME to my_malloc() to get error messages. Catching return codes of added memory allocation calls.
Whitespace changes to minimize diff against the main 5.0.
sql/log.cc
1.174 05/09/06 14:56:53 guilhem@stripped +15 -16
show_type[] was of no use (after I removed a DBUG_PRINT using it). Comments about questionable code place.
sql/handler.h
1.151 05/09/06 14:56:53 guilhem@stripped +2 -2
handler::rplf was not used
sql/handler.cc
1.191 05/09/06 14:56:53 guilhem@stripped +30 -29
catching return codes from all added function calls
# This is a BitKeeper patch. What follows are the unified diffs for the
# set of deltas contained in the patch. The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User: guilhem
# Host: gbichot3.local
# Root: /home/mysql_src/mysql-5.0-wl1012
--- 1.190/sql/handler.cc 2005-09-03 11:30:41 +02:00
+++ 1.191/sql/handler.cc 2005-09-06 14:56:53 +02:00
@@ -622,7 +622,11 @@
DBUG_RETURN(2);
}
#ifdef USING_TRANSACTIONS
- thd->prepare_for_commit(all); // TODO! catch the return code
+ if (unlikely(thd->prepare_for_commit(all)))
+ {
+ ha_rollback_trans(thd, all);
+ DBUG_RETURN(1);
+ }
if (trans->nht)
{
if (is_real_trans && wait_if_global_read_lock(thd, 0, 0))
@@ -735,7 +739,8 @@
DBUG_RETURN(1);
}
#ifdef USING_TRANSACTIONS
- thd->prepare_for_rollback(all);
+ if (unlikely(thd->prepare_for_rollback(all)))
+ DBUG_RETURN(1);
if (trans->nht)
{
/* Close all cursors that can not survive ROLLBACK */
@@ -2465,10 +2470,7 @@
A row in the given table should be replicated if:
- Row-based replication is on
- It is not a temporary table
- - The table shall be replicated
- - TODO: sort out why "mysql" is checked below; we want to be able to
- replicate CREATE PROCEDURE row-based (because it uses info which depends
- on user's context).
+ - The table shall be binlogged (binlog_*_db rules)
*/
#ifdef HAVE_ROW_BASED_REPLICATION
@@ -2484,6 +2486,7 @@
const byte *before_record,
byte *after_record)
{
+ bool error= 0;
if (check_table_binlog_row_based(table))
{
MY_BITMAP cols;
@@ -2491,20 +2494,22 @@
uchar bitbuf[BITMAP_STACKBUF_SIZE];
uint n_fields= table->s->fields;
my_bool use_bitbuf= n_fields <= sizeof(bitbuf)*8;
- bitmap_init(&cols,
- use_bitbuf ? bitbuf : NULL,
- (n_fields + 7) & ~7UL,
- false);
- bitmap_set_all(&cols);
- /* TODO: catch return code below */
- RowsEventT::binlog_row_logging_function(current_thd, table,
- table->file->has_transactions(),
- &cols, table->s->fields,
- before_record, after_record);
- if (!use_bitbuf)
- bitmap_free(&cols);
+ if (likely(!(error= bitmap_init(&cols,
+ use_bitbuf ? bitbuf : NULL,
+ (n_fields + 7) & ~7UL,
+ false))))
+ {
+ bitmap_set_all(&cols);
+ error=
+ RowsEventT::binlog_row_logging_function(current_thd, table,
+ table->file->has_transactions(),
+ &cols, table->s->fields,
+ before_record, after_record);
+ if (!use_bitbuf)
+ bitmap_free(&cols);
+ }
}
- return 0;
+ return error;
}
@@ -2519,41 +2524,37 @@
#endif /* HAVE_ROW_BASED_REPLICATION */
-int handler::
-ha_write_row(byte *buf)
+int handler::ha_write_row(byte *buf)
{
int error;
if (likely(!(error= write_row(buf))))
{
#ifdef HAVE_ROW_BASED_REPLICATION
- /* TODO catch return code here and in 2 next */
- binlog_log_row<Write_rows_log_event>(table, 0, buf);
+ error= binlog_log_row<Write_rows_log_event>(table, 0, buf);
#endif
}
return error;
}
-int handler::
-ha_update_row(const byte *old_data, byte *new_data)
+int handler::ha_update_row(const byte *old_data, byte *new_data)
{
int error;
if (likely(!(error= update_row(old_data, new_data))))
{
#ifdef HAVE_ROW_BASED_REPLICATION
- binlog_log_row<Update_rows_log_event>(table, old_data, new_data);
+ error= binlog_log_row<Update_rows_log_event>(table, old_data, new_data);
#endif
}
return error;
}
-int handler::
-ha_delete_row(const byte *buf)
+int handler::ha_delete_row(const byte *buf)
{
int error;
if (likely(!(error= delete_row(buf))))
{
#ifdef HAVE_ROW_BASED_REPLICATION
- binlog_log_row<Delete_rows_log_event>(table, buf, 0);
+ error= binlog_log_row<Delete_rows_log_event>(table, buf, 0);
#endif
}
return error;
--- 1.150/sql/handler.h 2005-09-05 11:33:42 +02:00
+++ 1.151/sql/handler.h 2005-09-06 14:56:53 +02:00
@@ -509,8 +509,8 @@
bool implicit_emptied; /* Can be !=0 only if HEAP */
const COND *pushed_cond;
- handler(const handlerton *ht_arg, TABLE *table_arg, ulong rplf= 0UL)
- :table(table_arg), ht(ht_arg),
+ handler(const handlerton *ht_arg, TABLE *table_arg) :table(table_arg),
+ ht(ht_arg),
ref(0), data_file_length(0), max_data_file_length(0), index_file_length(0),
delete_length(0), auto_increment_value(0),
records(0), deleted(0), mean_rec_length(0),
--- 1.173/sql/log.cc 2005-09-05 11:16:49 +02:00
+++ 1.174/sql/log.cc 2005-09-06 14:56:53 +02:00
@@ -76,15 +76,6 @@
return &binlog_hton;
}
-static char const* show_type[] = {
- "READ_CACHE",
- "WRITE_CACHE",
- "SEQ_READ_APPEND", /* sequential read or append */
- "READ_FIFO",
- "READ_NET",
- "WRITE_NET"
-};
-
static int binlog_close_connection(THD *thd)
{
IO_CACHE *trans_log= (IO_CACHE*)thd->ha_data[binlog_hton.slot];
@@ -1585,7 +1576,7 @@
bool MYSQL_LOG::write(Log_event *event_info)
{
- THD *thd=event_info->thd;
+ THD *thd= event_info->thd;
bool error= 1;
DBUG_ENTER("MYSQL_LOG::write(Log_event *)");
@@ -1683,11 +1674,15 @@
of the SQL command
*/
+ if (thd
#ifdef HAVE_ROW_BASED_REPLICATION
- if (!binlog_row_based && thd)
-#else
- if (thd)
+ /*
+ If row-based binlogging, Insert_id, Rand and other kind of "setting
+ context" events are not needed.
+ */
+ && !binlog_row_based
#endif
+ )
{
if (thd->last_insert_id_used)
{
@@ -2297,8 +2292,7 @@
#ifndef MYSQL_CLIENT
-ulong MYSQL_LOG::
-get_table_id(TABLE* table)
+ulong MYSQL_LOG::get_table_id(TABLE* table)
{
DBUG_ENTER("MYSQL_LOG::get_table_id(TABLE*)");
DBUG_PRINT("enter", ("table=%p", table));
@@ -2309,7 +2303,12 @@
pthread_mutex_lock(&LOCK_next_table_id);
/*
need to double check under mutex that no other thread has snuck in
- and set the table_map_id
+ and set the table_map_id.
+ Guilhem asks: so first we check without mutex. Imagine the == was false,
+ and another thread just made it true with mutex. It may be that the
+ first == test still sees that == is false, thus not even entering the
+ second == test. I wonder if this is ok. I hope something in this
+ scenario makes it impossible to happen?
*/
if ((tid= table->s->table_map_id) == table_mapping::NO_TABLE)
{
--- 1.195/sql/log_event.cc 2005-09-05 11:33:42 +02:00
+++ 1.196/sql/log_event.cc 2005-09-06 14:56:53 +02:00
@@ -26,7 +26,6 @@
#include "rpl_filter.h"
#include <my_dir.h>
#endif /* MYSQL_CLIENT */
-
#include <my_bitmap.h>
#include <my_vle.h>
@@ -465,6 +464,7 @@
DBUG_RETURN(0);
}
+
/*
Log_event::pack_info()
*/
@@ -4885,6 +4885,10 @@
m_tbllen(strlen(tbl_arg->s->table_name)),
m_table_id(tid),
m_width(tbl_arg->s->fields),
+ /*
+ TODO: allocating several kB for maybe no use (e.g. read-only session)
+ is too much.
+ */
m_rows_buf(my_malloc(opt_binlog_rows_event_max_size * sizeof(*m_rows_buf), MYF(MY_WME))),
m_rows_cur(m_rows_buf),
m_rows_end(m_rows_buf + opt_binlog_rows_event_max_size),
@@ -4894,12 +4898,12 @@
set_flags(NO_FOREIGN_KEY_CHECKS_F);
if (thd_arg->options & OPTION_RELAXED_UNIQUE_CHECKS)
set_flags(RELAXED_UNIQUE_CHECKS_F);
-
- bitmap_init(&m_cols,
- m_width <= sizeof(m_bitbuf)*8 ? m_bitbuf : NULL,
- (m_width + 7) & ~7UL,
- false);
- memcpy(m_cols.bitmap, cols->bitmap, byte_count(cols));
+ /* if bitmap_init fails, catched in is_valid() */
+ if (likely(!bitmap_init(&m_cols,
+ m_width <= sizeof(m_bitbuf)*8 ? m_bitbuf : NULL,
+ (m_width + 7) & ~7UL,
+ false)))
+ memcpy(m_cols.bitmap, cols->bitmap, byte_count(cols));
}
#endif
@@ -4938,15 +4942,15 @@
DBUG_PRINT("info",("m_table_id=%d, m_flags=%d, m_width=%u, data_size=%lu",
m_table_id, m_flags, m_width, data_size));
- m_rows_buf= my_malloc(data_size, MYF(0));
+ m_rows_buf= my_malloc(data_size, MYF(MY_WME));
if (m_rows_buf)
{
- bitmap_init(&m_cols,
- m_width <= sizeof(m_bitbuf)*8 ? m_bitbuf : NULL,
- (m_width + 7) & ~7UL,
- false);
- memcpy(m_cols.bitmap, ptr_width + 1, byte_count);
-
+ /* if bitmap_init fails, catched in is_valid() */
+ if (likely(!bitmap_init(&m_cols,
+ m_width <= sizeof(m_bitbuf)*8 ? m_bitbuf : NULL,
+ (m_width + 7) & ~7UL,
+ false)))
+ memcpy(m_cols.bitmap, ptr_width + 1, byte_count);
m_rows_end= m_rows_buf + data_size;
m_rows_cur= m_rows_end;
memcpy(m_rows_buf, ptr_rows_data, data_size);
@@ -4960,7 +4964,7 @@
my_free(m_rows_buf, MYF(MY_ALLOW_ZERO_PTR));
}
-void Rows_log_event::do_add_row_data(byte *const row_data,
+bool Rows_log_event::do_add_row_data(byte *const row_data,
my_size_t const length)
{
/*
@@ -4983,6 +4987,8 @@
my_ptrdiff_t const cur_size= m_rows_cur - m_rows_buf;
byte* const new_buf= my_realloc(m_rows_buf, new_alloc, MYF(MY_WME));
+ if (unlikely(!new_buf))
+ DBUG_RETURN(1);
/* If the memory moved, we need to move the pointers */
if (new_buf != m_rows_buf)
@@ -5001,7 +5007,7 @@
DBUG_ASSERT(m_rows_cur + length < m_rows_end);
memcpy(m_rows_cur, row_data, length);
m_rows_cur+= length;
- DBUG_VOID_RETURN;
+ DBUG_RETURN(0);
}
#ifndef MYSQL_CLIENT
@@ -5264,7 +5270,6 @@
m_tblnam(tbl->s->table_name),
m_tbllen(strlen(m_tblnam)),
m_colcnt(tbl->s->fields), m_coltype(0),
- m_memory(0),
m_table_id(tid),
m_flags(flags)
{
@@ -5273,6 +5278,7 @@
m_data_size+= m_tbllen + 2; // Include length and terminating \0
m_data_size+= 1 + m_colcnt; // COLCNT and column types
+ /* If malloc fails, catched in is_valid() */
if ((m_memory= m_coltype= my_malloc(m_colcnt, MYF(MY_WME))))
{
for (unsigned int i= 0 ; i < m_table->s->fields ; ++i)
@@ -5329,8 +5335,8 @@
m_dblen, ptr_dblen-vpart, m_tbllen, ptr_tbllen-vpart,
m_colcnt, ptr_colcnt-vpart));
- /* Allocate memory for all fields in one go */
- m_memory= my_multi_malloc(MYF(0),
+ /* Allocate mem for all fields in one go. If fails, catched in is_valid() */
+ m_memory= my_multi_malloc(MYF(MY_WME),
&m_dbnam, m_dblen + 1,
&m_tblnam, m_tbllen + 1,
&m_coltype, m_colcnt,
@@ -5765,7 +5771,7 @@
int error= 0;
my_size_t const keylen= table->s->keys > 0 ? table->key_info->key_length : 1;
m_memory=
- my_multi_malloc(MYF(0),
+ my_multi_malloc(MYF(MY_WME),
&m_search_record, table->s->reclength,
/* Just to get a sensible value when there are no keys */
&m_key, keylen,
@@ -5915,7 +5921,7 @@
int error= 0;
my_size_t const keylen= table->s->keys > 0 ? table->key_info->key_length : 1;
m_memory=
- my_multi_malloc(MYF(0),
+ my_multi_malloc(MYF(MY_WME),
&m_search_record, table->s->reclength,
/* Just to get a sensible value when there are no keys */
&m_key, keylen,
--- 1.120/sql/log_event.h 2005-09-03 11:30:41 +02:00
+++ 1.121/sql/log_event.h 2005-09-06 14:56:53 +02:00
@@ -1608,8 +1608,8 @@
Table map log event class
- Create a mapping from an table name and a database name to a table
- identifier.
+ Create a mapping from a (database name, table name) couple to a table
+ identifier (an integer number).
****************************************************************************/
@@ -1640,7 +1640,7 @@
void set_flags(flag_set flag) { m_flags |= flag; }
void clear_flags(flag_set flag) { m_flags &= ~flag; }
- flag_set get_flags(flag_set flag) { return m_flags & flag; }
+ flag_set get_flags(flag_set flag) const { return m_flags & flag; }
#ifndef MYSQL_CLIENT
Table_map_log_event(THD *thd, TABLE *tbl, ulong tid,
@@ -1654,7 +1654,7 @@
~Table_map_log_event();
virtual Log_event_type get_type_code() { return TABLE_MAP_EVENT; }
- virtual bool is_valid() const { return 1; }
+ virtual bool is_valid() const { return m_memory; /* we check malloc */ }
virtual int get_data_size() { return m_data_size; }
#ifndef MYSQL_CLIENT
@@ -1740,7 +1740,7 @@
void set_flags(flag_set flags) { m_flags |= flags; }
void clear_flags(flag_set flags) { m_flags &= ~flags; }
- flag_set get_flags(flag_set flags) { return m_flags & flags; }
+ flag_set get_flags(flag_set flags) const { return m_flags & flags; }
#ifndef MYSQL_CLIENT
#ifdef HAVE_REPLICATION
@@ -1753,8 +1753,9 @@
LAST_EVENT_INFO *last_event_info= 0);
#endif
- void add_row_data(byte *data, my_size_t length) {
- do_add_row_data(data,length);
+ bool add_row_data(byte *data, my_size_t length)
+ {
+ return do_add_row_data(data,length);
}
/* Member functions to implement superclass interface */
@@ -1771,6 +1772,11 @@
virtual bool write_data_header(IO_CACHE *file);
virtual bool write_data_body(IO_CACHE *file);
virtual const char *get_db() { return m_dbnam; }
+ virtual bool is_valid() const
+ {
+ /* that's how we check malloc() succeeded */
+ return m_rows_buf && m_cols.bitmap;
+ }
protected:
/*
@@ -1788,11 +1794,12 @@
/*
Helper function to count the number of bytes in a bitmap.
*/
- static my_size_t byte_count(MY_BITMAP const *const bits) {
- return bits->bitmap_size;
+ static my_size_t byte_count(MY_BITMAP const *const bits)
+ {
+ return bits->bitmap_size;
}
- virtual void do_add_row_data(byte *data, my_size_t length);
+ virtual bool do_add_row_data(byte *data, my_size_t length);
char const *m_dbnam; /* Database name */
my_size_t m_dblen; /* Length of database name in bytes */
@@ -1927,7 +1934,6 @@
TODO: it should not always return 1 but rather perform checks on the event
(could all members be alloced etc).
*/
- virtual bool is_valid() const { return 1; }
#ifdef MYSQL_CLIENT
void print(FILE *file, bool short_form, LAST_EVENT_INFO *last_event_info);
@@ -1991,7 +1997,6 @@
private:
virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
- virtual bool is_valid() const { return 1; }
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
gptr m_memory;
@@ -2057,7 +2062,6 @@
private:
virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
- virtual bool is_valid() const { return 1; }
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
gptr m_memory;
--- 1.202/sql/sql_class.cc 2005-09-05 11:16:49 +02:00
+++ 1.203/sql/sql_class.cc 2005-09-06 14:56:53 +02:00
@@ -2084,6 +2084,9 @@
Rows_log_event* pending= binlog_get_pending_rows_event();
+ if (unlikely(!pending->is_valid()))
+ return NULL;
+
/*
Check if the current event is non-NULL and a write-rows
event. Also check if the table provided is mapped: if it is not,
@@ -2299,8 +2302,8 @@
return 1;
row_data= table->write_row_record;
}
- else
- row_data= my_malloc(max_len, MYF(MY_WME));
+ else if (unlikely(!(row_data= my_malloc(max_len, MYF(MY_WME)))))
+ return 1;
}
my_size_t const len= pack_row(table, cols, row_data, max_len, record);
@@ -2308,13 +2311,8 @@
ev= binlog_prepare_pending_rows_event<Write_rows_log_event>
(table, server_id, cols, colcnt, len, is_trans);
- if (likely(ev != 0))
- {
- /* add_row_data copies row_data to internal buffer */
- ev->add_row_data(row_data,len);
- }
- else
- error= 1;
+ /* add_row_data copies row_data to internal buffer */
+ error= (unlikely(!ev)) || ev->add_row_data(row_data,len);
if (table->write_row_record == 0)
my_free(row_data, MYF(MY_WME));
@@ -2333,19 +2331,20 @@
my_size_t const before_maxlen = max_row_length(table, before_record);
my_size_t const after_maxlen = max_row_length(table, after_record);
- byte *memory= table->write_row_record;
+ byte *row_data= table->write_row_record;
byte *before_row, *after_row;
- if (memory != 0)
+ if (row_data != 0)
{
- before_row= memory;
- after_row= before_row+before_maxlen;
+ before_row= row_data;
+ after_row= before_row + before_maxlen;
}
else
{
- memory= my_multi_malloc(MYF(MY_WME),
- &before_row, before_maxlen,
- &after_row, after_maxlen,
- NULL);
+ if (unlikely(!(row_data= my_multi_malloc(MYF(MY_WME),
+ &before_row, before_maxlen,
+ &after_row, after_maxlen,
+ NULL))))
+ return 1;
}
my_size_t const before_size= pack_row(table, cols, before_row,
@@ -2357,18 +2356,13 @@
ev= binlog_prepare_pending_rows_event<Update_rows_log_event>
(table, server_id, cols, colcnt, before_size + after_size, is_trans);
- if (likely(ev != 0))
- {
- ev->add_row_data(before_row, before_size);
+ error= (unlikely(!ev)) || ev->add_row_data(before_row, before_size) ||
ev->add_row_data(after_row, after_size);
- }
- else
- error= 1;
if (!table->write_row_record)
{
/* add_row_data copies row_data to internal buffer */
- my_free(memory, MYF(MY_WME));
+ my_free(row_data, MYF(MY_WME));
}
return error;
@@ -2387,18 +2381,16 @@
bool error= 0;
my_size_t const max_len= max_row_length(table, record);
byte *row_data= table->write_row_record;
- if (!row_data)
- row_data= my_malloc(max_len, MYF(MY_WME));
+ if (!row_data && unlikely(!(row_data= my_malloc(max_len, MYF(MY_WME)))))
+ return 1;
my_size_t const len= pack_row(table, cols, row_data, max_len, record);
Rows_log_event* const
ev= binlog_prepare_pending_rows_event<Delete_rows_log_event>
(table, server_id, cols, colcnt, len, is_trans);
- if (likely(ev != 0))
- ev->add_row_data(row_data, len);
- else
- error= 1;
+ error= (unlikely(!ev)) || ev->add_row_data(row_data, len);
+
/* add_row_data copies row_data */
if (table->write_row_record == 0)
my_free(row_data, MYF(MY_WME));
--- 1.257/sql/sql_class.h 2005-09-03 11:30:41 +02:00
+++ 1.258/sql/sql_class.h 2005-09-06 14:56:53 +02:00
@@ -1290,7 +1290,8 @@
This function will be called from ha_rollback_trans() to prepare for a
rollback. The thread should take all necessary actions to prepare itself
for rollback; presently it's writing the last buffered row-based binlog
- event to the binlog (so that they do get thrown away if needed).
+ event to the binlog transaction cache (so that they do get thrown away if
+ needed).
*/
bool prepare_for_rollback(bool all) {
#if !defined(MYSQL_CLIENT) && defined(HAVE_ROW_BASED_REPLICATION)
| Thread |
|---|
| • bk commit into 5.0 tree (guilhem:1.1935) | guilhem | 6 Sep |
