bk commit into 5.0 tree (cmiller:1.2420)

[Chad MILLER]

Below is the list of changes that have just been committed into a local
5.0 repository of cmiller. When cmiller 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, 2007-03-02 10:32:12-05:00, cmiller@stripped +9 -0
  Update areas of the code that expected THD::options to be 32 bit, 
  when it's been 64 bit since mid-2005.
  
  This adds only comments to log_events, which cherry-picks a subset
  of the flags to pass, and avoids bugs because those bits are below
  2**32.

  BitKeeper/etc/collapsed@stripped, 2007-03-02 10:28:44-05:00, cmiller@stripped +1
-0

  sql/ha_berkeley.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +2 -2
    THD::options is a ulonglong.

  sql/log_event.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +8 -2
    Add comments noting problems in the code.

  sql/log_event.h@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +2 -0
    Add comments noting problems in the code.

  sql/mysql_priv.h@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +4 -0
    Add a comment to warn others that sql/log_event.* may have dangerous 
    code.

  sql/set_var.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +2 -2
    THD::options is a ulonglong.  Discard harmful casts.

  sql/sql_base.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +3 -3
    THD::options is a ulonglong.  Remove harmful casts.

  sql/sql_delete.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +1 -1
    THD::options is a ulonglong.  Remove harmful casts.

  sql/sql_parse.cc@stripped, 2007-03-02 10:32:10-05:00, cmiller@stripped +10 -10
    THD::options is a ulonglong.  Remove harmful casts.

# 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:	cmiller
# Host:	zippy.cornsilk.net
# Root:	/home/cmiller/work/mysql/mysql-5.0-maint

--- 1.165/sql/ha_berkeley.cc	2007-03-02 10:32:17 -05:00
+++ 1.166/sql/ha_berkeley.cc	2007-03-02 10:32:17 -05:00
@@ -1855,8 +1855,8 @@ int ha_berkeley::external_lock(THD *thd,
                            OPTION_TABLE_LOCK)) && !trx->all)
       {
 	/* We have to start a master transaction */
-	DBUG_PRINT("trans",("starting transaction all:  options: 0x%lx",
-                            (ulong) thd->options));
+	DBUG_PRINT("trans",("starting transaction all:  options: 0x%llx",
+                            thd->options));
         if ((error=txn_begin(db_env, 0, &trx->all, 0)))
 	{
           trx->bdb_lock_count--;        // We didn't get the lock

--- 1.223/sql/log_event.cc	2007-03-02 10:32:17 -05:00
+++ 1.224/sql/log_event.cc	2007-03-02 10:32:17 -05:00
@@ -1082,7 +1082,7 @@ static void write_str_with_code_and_len(
 bool Query_log_event::write(IO_CACHE* file)
 {
   uchar buf[QUERY_HEADER_LEN+
-            1+4+           // code of flags2 and flags2
+            1+4+           // code of flags2 and flags2;  FIXME 64 bits?
             1+8+           // code of sql_mode and sql_mode
             1+1+FN_REFLEN+ // code of catalog and catalog length and catalog
             1+4+           // code of autoinc and the 2 autoinc variables
@@ -1146,7 +1146,7 @@ bool Query_log_event::write(IO_CACHE* fi
   if (flags2_inited)
   {
     *start++= Q_FLAGS2_CODE;
-    int4store(start, flags2);
+    int4store(start, flags2);  /* FIXME? isn't thd->options 8 bytes? */
     start+= 4;
   }
   if (sql_mode_inited)
@@ -1280,14 +1280,17 @@ Query_log_event::Query_log_event(THD* th
   catalog_len = (catalog) ? (uint32) strlen(catalog) : 0;
   /* status_vars_len is set just before writing the event */
   db_len = (db) ? (uint32) strlen(db) : 0;
+
   /*
     If we don't use flags2 for anything else than options contained in
     thd->options, it would be more efficient to flags2=thd_arg->options
     (OPTIONS_WRITTEN_TO_BINLOG would be used only at reading time).
     But it's likely that we don't want to use 32 bits for 3 bits; in the future
     we will probably want to reclaim the 29 bits. So we need the &.
+    FIXME:  But isn't thd->options 8 bytes? -cm
   */
   flags2= (uint32) (thd_arg->options & OPTIONS_WRITTEN_TO_BIN_LOG);
+
   DBUG_ASSERT(thd->variables.character_set_client->number < 256*256);
   DBUG_ASSERT(thd->variables.collation_connection->number < 256*256);
   DBUG_ASSERT(thd->variables.collation_server->number < 256*256);
@@ -1306,6 +1309,7 @@ Query_log_event::Query_log_event(THD* th
   }
   else
     time_zone_len= 0;
+  /* FIXME: flags2 is uint32 while thd->options is ulonglong -cm */
   DBUG_PRINT("info",("Query_log_event has flags2: %lu  sql_mode: %lu",
                      (ulong) flags2, sql_mode));
 }
@@ -1409,6 +1413,7 @@ Query_log_event::Query_log_event(const c
     switch (*pos++) {
     case Q_FLAGS2_CODE:
       flags2_inited= 1;
+      /* FIXME: flags2 is 4 bytes while thd->options is 8 bytes */
       flags2= uint4korr(pos);
       DBUG_PRINT("info",("In Query_log_event, read flags2: %lu", (ulong) flags2));
       pos+= 4;
@@ -1752,6 +1757,7 @@ int Query_log_event::exec_event(struct s
           all bits of thd->options which are 1 in OPTIONS_WRITTEN_TO_BIN_LOG must
           take their value from flags2.
         */
+        /* FIXME!  Here, we lose bits!  options is 64 bits and we set 32! -cm */
         thd->options= flags2|(thd->options &
~(ulong)OPTIONS_WRITTEN_TO_BIN_LOG);
       /*
         else, we are in a 3.23/4.0 binlog; we previously received a

--- 1.130/sql/log_event.h	2007-03-02 10:32:17 -05:00
+++ 1.131/sql/log_event.h	2007-03-02 10:32:17 -05:00
@@ -203,6 +203,7 @@ struct sql_ex_info
   Max number of possible extra bytes in a replication event compared to a
   packet (i.e. a query) sent from client to master;
   First, an auxiliary log_event status vars estimation:
+  FIXME, flags2 represents thd->options, which is 64 bits.  May cause problems.
 */
 #define MAX_SIZE_LOG_EVENT_STATUS (4 /* flags2 */   + \
                                    8 /* sql mode */ + \
@@ -501,6 +502,7 @@ typedef struct st_print_event_info
   // TODO: have the last catalog here ??
   char db[FN_REFLEN+1]; // TODO: make this a LEX_STRING when thd->db is
   bool flags2_inited;
+  /* FIXME: This represents thd->options, which is 64 bits. */
   uint32 flags2;
   bool sql_mode_inited;
   ulong sql_mode;		/* must be same as THD.variables.sql_mode */

--- 1.437/sql/mysql_priv.h	2007-03-02 10:32:17 -05:00
+++ 1.438/sql/mysql_priv.h	2007-03-02 10:32:17 -05:00
@@ -355,6 +355,10 @@ MY_LOCALE *my_locale_by_number(uint numb
   fulltext functions when reading from it.
 */
 #define TMP_TABLE_FORCE_MYISAM          (ULL(1) << 32)
+/*
+  BEWARE that some parts of the code still expect only 32 bits for 
+  thd->options .
+*/
 
 
 /*

--- 1.365/sql/sql_base.cc	2007-03-02 10:32:17 -05:00
+++ 1.366/sql/sql_base.cc	2007-03-02 10:32:17 -05:00
@@ -549,7 +549,7 @@ void close_thread_tables(THD *thd, bool 
       good idea to turn off OPTION_TABLE_LOCK flag.
     */
     DBUG_ASSERT(thd->lex->requires_prelocking());
-    thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+    thd->options&= ~(OPTION_TABLE_LOCK);
   }
 
   DBUG_VOID_RETURN;
@@ -2611,7 +2611,7 @@ int lock_tables(THD *thd, TABLE_LIST *ta
     {
       if (thd->lex->requires_prelocking())
       {
-        thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+        thd->options&= ~(OPTION_TABLE_LOCK);
         thd->in_lock_tables=0;
       }
       DBUG_RETURN(-1);
@@ -2644,7 +2644,7 @@ int lock_tables(THD *thd, TABLE_LIST *ta
             ha_rollback_stmt(thd);
             mysql_unlock_tables(thd, thd->locked_tables);
             thd->locked_tables= 0;
-            thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+            thd->options&= ~(OPTION_TABLE_LOCK);
             DBUG_RETURN(-1);
           }
         }

--- 1.191/sql/sql_delete.cc	2007-03-02 10:32:17 -05:00
+++ 1.192/sql/sql_delete.cc	2007-03-02 10:32:17 -05:00
@@ -907,7 +907,7 @@ end:
   /* Probably InnoDB table */
   ulonglong save_options= thd->options;
   table_list->lock_type= TL_WRITE;
-  thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_NOT_AUTOCOMMIT);
+  thd->options&= ~(OPTION_BEGIN | OPTION_NOT_AUTOCOMMIT);
   ha_enable_transaction(thd, FALSE);
   mysql_init_select(thd->lex);
   error= mysql_delete(thd, table_list, (COND*) 0, (SQL_LIST*) 0,

--- 1.604/sql/sql_parse.cc	2007-03-02 10:32:17 -05:00
+++ 1.605/sql/sql_parse.cc	2007-03-02 10:32:17 -05:00
@@ -140,14 +140,14 @@ static bool end_active_trans(THD *thd)
   if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN |
 		      OPTION_TABLE_LOCK))
   {
-    DBUG_PRINT("info",("options: 0x%lx", (ulong) thd->options));
+    DBUG_PRINT("info",("options: 0x%llx", thd->options));
     /* Safety if one did "drop table" on locked tables */
     if (!thd->locked_tables)
       thd->options&= ~OPTION_TABLE_LOCK;
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     if (ha_commit(thd))
       error=1;
-    thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+    thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
   }
   DBUG_RETURN(error);
 }
@@ -171,7 +171,7 @@ static bool begin_trans(THD *thd)
   else
   {
     LEX *lex= thd->lex;
-    thd->options= ((thd->options & (ulong) ~(OPTION_STATUS_NO_TRANS_UPDATE)) |
+    thd->options= ((thd->options & ~(OPTION_STATUS_NO_TRANS_UPDATE)) |
 		   OPTION_BEGIN);
     thd->server_status|= SERVER_STATUS_IN_TRANS;
     if (lex->start_transaction_opt & MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT)
@@ -1454,7 +1454,7 @@ int end_trans(THD *thd, enum enum_mysql_
     */
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     res= ha_commit(thd);
-    thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+    thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
     break;
   case COMMIT_RELEASE:
     do_release= 1; /* fall through */
@@ -1471,7 +1471,7 @@ int end_trans(THD *thd, enum enum_mysql_
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     if (ha_rollback(thd))
       res= -1;
-    thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+    thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
     if (!res && (completion == ROLLBACK_AND_CHAIN))
       res= begin_trans(thd);
     break;
@@ -3778,7 +3778,7 @@ end_with_restore_list:
     if (thd->options & OPTION_TABLE_LOCK)
     {
       end_active_trans(thd);
-      thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+      thd->options&= ~(OPTION_TABLE_LOCK);
     }
     if (thd->global_read_lock)
       unlock_global_read_lock(thd);
@@ -3804,7 +3804,7 @@ end_with_restore_list:
       send_ok(thd);
     }
     else
-      thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+      thd->options&= ~(OPTION_TABLE_LOCK);
     thd->in_lock_tables=0;
     break;
   case SQLCOM_CREATE_DB:
@@ -4911,7 +4911,7 @@ create_sp_error:
     thd->transaction.xid_state.xa_state=XA_ACTIVE;
     thd->transaction.xid_state.xid.set(thd->lex->xid);
     xid_cache_insert(&thd->transaction.xid_state);
-    thd->options= ((thd->options & (ulong) ~(OPTION_STATUS_NO_TRANS_UPDATE)) |
+    thd->options= ((thd->options & ~(OPTION_STATUS_NO_TRANS_UPDATE)) |
                    OPTION_BEGIN);
     thd->server_status|= SERVER_STATUS_IN_TRANS;
     send_ok(thd);
@@ -5005,7 +5005,7 @@ create_sp_error:
                xa_state_names[thd->transaction.xid_state.xa_state]);
       break;
     }
-    thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+    thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     xid_cache_delete(&thd->transaction.xid_state);
     thd->transaction.xid_state.xa_state=XA_NOTR;
@@ -5035,7 +5035,7 @@ create_sp_error:
       my_error(ER_XAER_RMERR, MYF(0));
     else
       send_ok(thd);
-    thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+    thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     xid_cache_delete(&thd->transaction.xid_state);
     thd->transaction.xid_state.xa_state=XA_NOTR;

--- 1.25/BitKeeper/etc/collapsed	2007-03-02 10:32:17 -05:00
+++ 1.26/BitKeeper/etc/collapsed	2007-03-02 10:32:17 -05:00
@@ -41,3 +41,4 @@
 45c0fdfb2mz6NdOIsLenJtf6_ZelTA
 45d1ffcd-r3v8A7uh92hQaMfQM9UPQ
 45d21437Vg_-i4uOWyvzYWHESXDP6A
+45dcdcccB88yyMDP6NfOxMhyBdZ0iA

--- 1.179/sql/set_var.cc	2007-03-02 10:32:17 -05:00
+++ 1.180/sql/set_var.cc	2007-03-02 10:32:17 -05:00
@@ -2868,14 +2868,14 @@ static bool set_option_autocommit(THD *t
     if ((org_options & OPTION_NOT_AUTOCOMMIT))
     {
       /* We changed to auto_commit mode */
-      thd->options&= ~(ulong) (OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
+      thd->options&= ~(OPTION_BEGIN | OPTION_STATUS_NO_TRANS_UPDATE);
       thd->server_status|= SERVER_STATUS_AUTOCOMMIT;
       if (ha_commit(thd))
 	return 1;
     }
     else
     {
-      thd->options&= ~(ulong) (OPTION_STATUS_NO_TRANS_UPDATE);
+      thd->options&= ~(OPTION_STATUS_NO_TRANS_UPDATE);
       thd->server_status&= ~SERVER_STATUS_AUTOCOMMIT;
     }
   }