Review of all code pushed since last review

Simple optimzations and cleanups
Removed compiler warnings and fixed portability issues
Added client functions 'mysql_embedded()' to allow client to check if we are using embedded server
Fixes for purify


client/mysqlimport.c:
  Remove not used variable
client/mysqltest.c:
  Remove usage of MAXPATHLEN (all MySQL code uses FN_REFLEN)
  Simplified code
  Remove usage of sprintf("%llu") as this is not portable
include/mysql.h:
  Added mysql_embedded() to be able to easily check if we are using the embedded server
innobase/srv/srv0start.c:
  Don't use memcmp() when using purify (to avoid false warnings)
libmysql/libmysql.c:
  Added mysql_embedded() to be able to easily check if we are using the embedded server
libmysql/libmysql.def:
  Added mysql_embedded() to be able to easily check if we are using the embedded server
myisam/myisam_ftdump.c:
  Remove compiler warning
myisam/myisamchk.c:
  Remove compiler warning
myisam/rt_test.c:
  #ifdef not used code
mysys/hash.c:
  Remove compiler warning (from last push)
mysys/my_gethwaddr.c:
  Remove compiler warning
ndb/src/ndbapi/ndberror.c:
  #ifdef not used code
regex/regcomp.c:
  Remove not used code
regex/regcomp.ih:
  Remove not used code (to remove compiler warnings)
sql-common/client.c:
  Remove compiler warnings
sql/field.cc:
  Simple optimization
sql/ha_innodb.cc:
  Rename mysql_embedded -> mysqld_embedded
sql/item.cc:
  Fix comments
  Move variables first on block
  Remove else after return
  Simple optimizations
  (no logic changes)
sql/item_cmpfunc.cc:
  Added comment
sql/mysql_priv.h:
  Rename mysql_embedded -> mysqld_embedded
sql/mysqld.cc:
  Rename mysql_embedded -> mysqld_embedded
sql/sql_acl.cc:
  Added comments
  simple optimization
  Fixed 'very unlikely' bug when doing REVOKE ALL PRIVILEGES
sql/sql_select.cc:
  More comments
  Simple optimization
sql/sql_show.cc:
  Simple changes to make similar code similar
  More comments
sql/sql_string.cc:
  Trivial optimization and better code layout
strings/Makefile.am:
  Change xml.c to use bcmp to avoid warnings from purify
strings/xml.c:
  Change xml.c to use bcmp to avoid warnings from purify
tests/client_test.c:
  Remove usage of MAXPATHLEN (all MySQL code uses FN_REFLEN)

10 files changed, 137 insertions, 97 deletions

diff --git a/sql/field.cc b/sql/field.cc
index 3dc1375dff3..e7e8ec736ef 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -304,14 +304,11 @@ bool Field::field_cast_compatible(Field::field_cast_enum type)
 {
   DBUG_ASSERT(type != FIELD_CAST_STOP);
   Field::field_cast_enum *array= field_cast_array[field_cast_type()];
-  uint i= 0;
-  Field::field_cast_enum tp;
-  do
+  while (*array != FIELD_CAST_STOP)
   {
-    tp=  array[i++];
-    if (tp == type)
+    if (*(array++) == type)
       return 1;
-  } while (tp != FIELD_CAST_STOP);
+  }
   return 0;
 }
 
@@ -4438,13 +4435,9 @@ char *Field_string::pack(char *to, const char *from, uint max_length)
   while (end > from && end[-1] == ' ')
     end--;
   length= (end-from);
+  *to++= (char) (uchar) length;
   if (field_length > 255)
-  {
-    int2store(to, length);
-    to+= 2;
-  }
-  else
-    *to++= (char) (uchar) length;
+    *to++= (char) (uchar) (length >> 8);
   memcpy(to, from, (int) length);
   return to+length;
 }
@@ -4459,13 +4452,9 @@ char *Field_string::pack_key(char *to, const char *from, uint max_length)
   set_if_smaller(length, char_length);
   while (length && from[length-1] == ' ')
     length--;
+  *to++= (char) (uchar) length;
   if (field_length > 255)
-  {
-    int2store(to, length);
-    to+= 2;
-  }
-  else
-    *to++= (char) (uchar) length;
+    *to++= (char) (uchar) (length >> 8);
   memcpy(to, from, length);
   return to+length;
 }
diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index d748c005d02..fbf46dd0a50 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -855,7 +855,7 @@ innobase_init(void)
 	Note that when using the embedded server, the datadirectory is not
 	necessarily the current directory of this program. */
 
-	if (mysql_embedded) {
+	if (mysqld_embedded) {
 		default_path = mysql_real_data_home;
 		fil_path_to_mysql_datadir = mysql_real_data_home;
 	} else {
@@ -993,7 +993,7 @@ innobase_init(void)
 	srv_max_n_open_files = (ulint) innobase_open_files;
 	srv_innodb_status = (ibool) innobase_create_status_file;
 
-	srv_print_verbose_log = mysql_embedded ? 0 : 1;
+	srv_print_verbose_log = mysqld_embedded ? 0 : 1;
 
 		/* Store the default charset-collation number of this MySQL
 	installation */
diff --git a/sql/item.cc b/sql/item.cc
index 0366ea29485..3bc65ca1062 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1196,7 +1196,7 @@ bool Item_ref_null_helper::get_date(TIME *ltime, uint fuzzydate)
 static void mark_as_dependent(THD *thd, SELECT_LEX *last, SELECT_LEX *current,
 			      Item_ident *item)
 {
-  // store pointer on SELECT_LEX from wich item is dependent
+  // store pointer on SELECT_LEX from which item is dependent
   item->depended_from= last;
   current->mark_as_dependent(last);
   if (thd->lex->describe & DESCRIBE_EXTENDED)
@@ -1872,10 +1872,11 @@ bool Item_field::send(Protocol *protocol, String *buffer)
   return protocol->store(result_field);
 }
 
+
 /*
   This is used for HAVING clause
   Find field in select list having the same name
- */
+*/
 
 bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 {
@@ -1904,8 +1905,9 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 				  REPORT_ALL_ERRORS ), &not_used)) ==
 	(Item **)not_found_item)
     {
-      upward_lookup= 1;
       Field *tmp= (Field*) not_found_field;
+      SELECT_LEX *last= 0;
+      upward_lookup= 1;
       /*
 	We can't find table field in select list of current select,
 	consequently we have to find it in outer subselect(s).
@@ -1915,7 +1917,6 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 	mention of table name, but if we join tables in one list it will
 	cause error ER_NON_UNIQ_ERROR in find_item_in_list.
       */
-      SELECT_LEX *last=0;
       for ( ; sl ; sl= (prev_unit= sl->master_unit())->outer_select())
       {
 	last= sl;
@@ -1967,9 +1968,9 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 
       if (!ref)
 	return 1;
-      else if (!tmp)
+      if (!tmp)
 	return -1;
-      else if (ref == (Item **)not_found_item && tmp == not_found_field)
+      if (ref == (Item **)not_found_item && tmp == not_found_field)
       {
 	if (upward_lookup)
 	{
@@ -1984,31 +1985,33 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference)
 			    *(thd->lex->current_select->get_item_list()),
 			    &counter, REPORT_ALL_ERRORS, &not_used);
 	}
-        ref= 0;
+        ref= 0;                                 // Safety
 	return 1;
       }
-      else if (tmp != not_found_field)
+      if (tmp != not_found_field)
       {
-	ref= 0; // To prevent "delete *ref;" on ~Item_ref() of this item
-	Item_field* fld= new Item_field(tmp);
-	if (!fld)
+        Item_field* fld;
+        /*
+          Set ref to 0 as we are replacing this item with the found item
+          and this will ensure we get an error if this item would be
+          used elsewhere
+        */
+	ref= 0;                                 // Safety
+	if (!(fld= new Item_field(tmp)))
 	  return 1;
 	thd->change_item_tree(reference, fld);
 	mark_as_dependent(thd, last, thd->lex->current_select, fld);
 	return 0;
       }
-      else
+      if (!(*ref)->fixed)
       {
-	if (!(*ref)->fixed)
-	{
-	  my_error(ER_ILLEGAL_REFERENCE, MYF(0), name,
-		   "forward reference in item list");
-	  return -1;
-	}
-	mark_as_dependent(thd, last, thd->lex->current_select,
-			  this);
-	ref= last->ref_pointer_array + counter;
+        my_error(ER_ILLEGAL_REFERENCE, MYF(0), name,
+                 "forward reference in item list");
+        return -1;
       }
+      mark_as_dependent(thd, last, thd->lex->current_select,
+                        this);
+      ref= last->ref_pointer_array + counter;
     }
     else if (!ref)
       return 1;
@@ -2523,12 +2526,13 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
   uint32 new_length= real_length(item);
   bool use_new_field= 0, use_expression_type= 0;
   Item_result new_result_type= type_convertor[item_type][item->result_type()];
+  bool item_is_a_field= item->type() == Item::FIELD_ITEM;
 
   /*
     Check if both items point to fields: in this case we
     can adjust column types of result table in the union smartly.
   */
-  if (field_example && item->type() == Item::FIELD_ITEM)
+  if (field_example && item_is_a_field)
   {
     Field *field= ((Item_field *)item)->field;
     /* Can 'field_example' field store data of the column? */
@@ -2545,23 +2549,25 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
          !is_attr_compatible(this, item));
     }
   }
-  else if (field_example || item->type() == Item::FIELD_ITEM)
+  else if (field_example || item_is_a_field)
   {
     /*
       Expression types can't be mixed with field types, we have to use
       expression types.
     */
+    use_new_field= 1;                           // make next if test easier
     use_expression_type= 1;
   }
 
   /* Check whether size/type of the result item should be changed */
-  if (use_new_field || use_expression_type ||
+  if (use_new_field ||
       (new_result_type != item_type) || (new_length > max_length) ||
       (!maybe_null && item->maybe_null) ||
       (item_type == STRING_RESULT &&
        !my_charset_same(collation.collation, item->collation.collation)))
   {
-    if (use_expression_type || item->type() != Item::FIELD_ITEM)
+    const char *old_cs,*old_derivation;
+    if (use_expression_type || !item_is_a_field)
       field_example= 0;
     else
     {
@@ -2572,8 +2578,8 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
       field_example= ((Item_field*) item)->field;
     }
 
-    const char *old_cs= collation.collation->name,
-      *old_derivation= collation.derivation_name();
+    old_cs= collation.collation->name;
+    old_derivation= collation.derivation_name();
     if (item_type == STRING_RESULT && collation.aggregate(item->collation))
     {
       my_error(ER_CANT_AGGREGATE_2COLLATIONS, MYF(0),
@@ -2593,12 +2599,12 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
   return 0;
 }
 
+
 uint32 Item_type_holder::real_length(Item *item)
 {
   if (item->type() == Item::FIELD_ITEM)
-  {
     return ((Item_field *)item)->max_disp_length();
-  }
+
   switch (item->result_type())
   {
   case STRING_RESULT:
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 248b56fe064..f6730c84979 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -859,6 +859,10 @@ void Item_func_between::fix_length_and_dec()
     Field *field=((Item_field*) args[0])->field;
     if (field->can_be_compared_as_longlong())
     {
+      /*
+        The following can't be recoded with || as convert_constant_item
+        changes the argument
+      */
       if (convert_constant_item(thd, field,&args[1]))
 	cmp_type=INT_RESULT;			// Works for all types.
       if (convert_constant_item(thd, field,&args[2]))
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 1a0879c6347..e94d436b135 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -897,7 +897,7 @@ extern uint test_flags,select_errors,ha_open_options;
 extern uint protocol_version, mysqld_port, dropping_tables;
 extern uint delay_key_write_options, lower_case_table_names;
 extern bool opt_endinfo, using_udf_functions, locked_in_memory;
-extern bool opt_using_transactions, mysql_embedded;
+extern bool opt_using_transactions, mysqld_embedded;
 extern bool using_update_log, opt_large_files, server_id_supplied;
 extern bool opt_log, opt_update_log, opt_bin_log, opt_slow_log, opt_error_log;
 extern bool opt_disable_networking, opt_skip_show_db;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index d7d0f637ec2..878b7bf7e92 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -449,9 +449,9 @@ pthread_cond_t eventShutdown;
 #endif
 
 #ifndef EMBEDDED_LIBRARY
-bool mysql_embedded=0;
+bool mysqld_embedded=0;
 #else
-bool mysql_embedded=1;
+bool mysqld_embedded=1;
 #endif
 
 #ifndef DBUG_OFF
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index fc252c1f5ac..b7eecac4e48 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -2398,7 +2398,8 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
 			    table_list->db,
 			    table_list->real_name,
 			    rights, column_priv, revoke_grant))
-    {						// Crashend table ??
+    {
+      /* Should only happen if table is crashed */
       result= -1;			       /* purecov: deadcode */
     }
     else if (tables[2].table)
@@ -3636,12 +3637,17 @@ int mysql_revoke_all(THD *thd,  List <LEX_USER> &list)
       if (!strcmp(lex_user->user.str,user) &&
 	  !my_strcasecmp(system_charset_info, lex_user->host.str, host))
       {
-	if (replace_db_table(tables[1].table, acl_db->db, *lex_user, ~0, 1))
-	  result= -1;
-	else
+	if (!replace_db_table(tables[1].table, acl_db->db, *lex_user, ~0, 1))
+        {
+          /*
+            Don't increment counter as replace_db_table deleted the
+            current element in acl_db's and shifted the higher elements down
+          */
 	  continue;
+        }
+        result= -1;                             // Something went wrong
       }
-      ++counter;
+      counter++;
     }
 
     /* Remove column access */
@@ -3664,26 +3670,27 @@ int mysql_revoke_all(THD *thd,  List <LEX_USER> &list)
 				~0, 0, 1))
 	{
 	  result= -1;
+          continue;
 	}
 	else
 	{
-	  if (grant_table->cols)
-	  {
-	    List<LEX_COLUMN> columns;
-	    if (replace_column_table(grant_table,tables[3].table, *lex_user,
-				     columns,
-				     grant_table->db,
-				     grant_table->tname,
-				     ~0, 1))
-	      result= -1;
-	    else
-	      continue;
-	  }
-	  else
-	    continue;
+	  if (!grant_table->cols)
+            continue;
+          List<LEX_COLUMN> columns;
+          if (replace_column_table(grant_table,tables[3].table, *lex_user,
+                                   columns,
+                                   grant_table->db,
+                                   grant_table->tname,
+                                   ~0, 1))
+          result= -1;
+          /*
+            Safer to do continue here as replace_table_table changed
+            column_priv_hash and we want to test the current element
+          */
+          continue;
 	}
       }
-      ++counter;
+      counter++;
     }
   }
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 638ed229a70..df74a946b5c 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -6911,10 +6911,23 @@ part_of_refkey(TABLE *table,Field *field)
 
 /*****************************************************************************
   Test if one can use the key to resolve ORDER BY
-  Returns: 1 if key is ok.
-	   0 if key can't be used
-	  -1 if reverse key can be used
-          used_key_parts is set to key parts used if length != 0
+
+  SYNOPSIS
+    test_if_order_by_key()
+    order		Sort order
+    table		Table to sort
+    idx			Index to check
+    used_key_parts	Return value for used key parts.
+
+
+  NOTES
+    used_key_parts is set to correct key parts used if return value != 0
+    (On other cases, used_key_part may be changed)
+
+  RETURN
+    1   key is ok.
+    0   Key can't be used
+    -1  Reverse key can be used
 *****************************************************************************/
 
 static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
@@ -6943,16 +6956,17 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
       DBUG_RETURN(0);
 
     /* set flag to 1 if we can use read-next on key, else to -1 */
-    flag= ((order->asc == !(key_part->key_part_flag & HA_REVERSE_SORT)) ? 1 : -1);
+    flag= ((order->asc == !(key_part->key_part_flag & HA_REVERSE_SORT)) ?
+           1 : -1);
     if (reverse && flag != reverse)
       DBUG_RETURN(0);
     reverse=flag;				// Remember if reverse
     key_part++;
   }
-  uint tmp= (uint) (key_part - table->key_info[idx].key_part);
-  if (reverse == -1 && !(table->file->index_flags(idx,tmp-1, 1) & HA_READ_PREV))
-    DBUG_RETURN(0);
-  *used_key_parts= tmp;
+  *used_key_parts= (uint) (key_part - table->key_info[idx].key_part);
+  if (reverse == -1 && !(table->file->index_flags(idx, *used_key_parts-1, 1) &
+                         HA_READ_PREV))
+    reverse= 0;                                 // Index can't be used
   DBUG_RETURN(reverse);
 }
 
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index c5cd2860e03..e030db2caf7 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -1136,7 +1136,7 @@ static const char *require_quotes(const char *name, uint name_length)
   for ( ; name < end ; name++)
   {
     uchar chr= (uchar) *name;
-    length= my_mbcharlen(system_charset_info, (uchar) chr);
+    length= my_mbcharlen(system_charset_info, chr);
     if (length == 1 && !system_charset_info->ident_map[chr])
       return name;
   }
@@ -1148,25 +1148,29 @@ void
 append_identifier(THD *thd, String *packet, const char *name, uint length)
 {
   const char *name_end;
+  char quote_char;
   int q= get_quote_char_for_identifier(thd, name, length);
 
-  if (q == EOF) {
+  if (q == EOF)
+  {
     packet->append(name, length, system_charset_info);
     return;
   }
 
-  char quote_char= q;
-
-  /* The identifier must be quoted as it includes a quote character */
+  /*
+    The identifier must be quoted as it includes a quote character or
+   it's a keyword
+  */
 
   packet->reserve(length*2 + 2);
+  quote_char= (char) q;
   packet->append(&quote_char, 1, system_charset_info);
 
   for (name_end= name+length ; name < name_end ; name+= length)
   {
-    char chr= *name;
-    length= my_mbcharlen(system_charset_info, (uchar) chr);
-    if (length == 1 && chr == quote_char)
+    uchar chr= (uchar) *name;
+    length= my_mbcharlen(system_charset_info, chr);
+    if (length == 1 && chr == (uchar) quote_char)
       packet->append(&quote_char, 1, system_charset_info);
     packet->append(name, length, packet->charset());
   }
@@ -1174,8 +1178,25 @@ append_identifier(THD *thd, String *packet, const char *name, uint length)
 }
 
 
-/* Get the quote character for displaying an identifier.
-   If no quote character is needed, return EOF. */
+/*
+  Get the quote character for displaying an identifier.
+
+  SYNOPSIS
+    get_quote_char_for_identifier()
+    thd		Thread handler
+    name	name to quote
+    length	length of name
+
+  IMPLEMENTATION
+    If name is a keyword or includes a special character, then force
+    quoting.
+    Otherwise identifier is quoted only if the option OPTION_QUOTE_SHOW_CREATE
+    is set.
+
+  RETURN
+    EOF	  No quote character is needed
+    #	  Quote character
+*/
 
 int get_quote_char_for_identifier(THD *thd, const char *name, uint length)
 {
@@ -1183,10 +1204,9 @@ int get_quote_char_for_identifier(THD *thd, const char *name, uint length)
       !require_quotes(name, length) &&
       !(thd->options & OPTION_QUOTE_SHOW_CREATE))
     return EOF;
-  else if (thd->variables.sql_mode & MODE_ANSI_QUOTES)
+  if (thd->variables.sql_mode & MODE_ANSI_QUOTES)
     return '"';
-  else
-    return '`';
+  return '`';
 }
 
 
@@ -1195,10 +1215,9 @@ int get_quote_char_for_identifier(THD *thd, const char *name, uint length)
 static void append_directory(THD *thd, String *packet, const char *dir_type,
 			     const char *filename)
 {
-  uint length;
   if (filename && !(thd->variables.sql_mode & MODE_NO_DIR_IN_CREATE))
   {
-    length= dirname_length(filename);
+    uint length= dirname_length(filename);
     packet->append(' ');
     packet->append(dir_type);
     packet->append(" DIRECTORY='", 12);
diff --git a/sql/sql_string.cc b/sql/sql_string.cc
index 1ec0faafa8f..1044dc4e58e 100644
--- a/sql/sql_string.cc
+++ b/sql/sql_string.cc
@@ -537,7 +537,8 @@ uint32 String::numchars()
 
 int String::charpos(int i,uint32 offset)
 {
-  if (i<0) return i;
+  if (i <= 0)
+    return i;
   return str_charset->cset->charpos(str_charset,Ptr+offset,Ptr+str_length,i);
 }