Merge lp:~chillycreator/drizzle/drizzle-fix into lp:~drizzle-trunk/drizzle/development

[chico chen]

change some "goto label;" to "return function();"

[Stewart Smith (stewart)]

I don't think this especially makes this bit of code easier to read/understand.

create_tmp_table() as a lot of problems, and the gotos aren't anywhere near the top.

the name error_return() is also too generic for a very specific error handling method.

that being said, the code does actually look correct, so good work on that!

review: Disapprove

[Monty Taylor (mordred)]

I agree with Stewart here. I'm not sure this particularly helps anything.

review: Disapprove

Unmerged revisions

1456. By chillyc <chillyc@chillyc-desktop> on 2010-04-09

change 'goto err;' to 'return error_return()', make code more OOM

=== modified file 'drizzled/table.cc'
--- drizzled/table.cc	2010-03-31 21:15:40 +0000
+++ drizzled/table.cc	2010-04-09 03:02:15 +0000
@@ -70,6 +70,7 @@
 void open_table_error(TableShare *share, int error, int db_errno,
                       myf errortype, int errarg);
 
+Table* error_return(memory::Root *mem_root_save,Session* session,Table* table);				
 /*************************************************************************/
 
 /* Get column name from column hash */
@@ -2351,6 +2352,14 @@
   internal::fn_format(path, path, drizzle_tmpdir, "", MY_REPLACE_EXT|MY_UNPACK_FILENAME);
 }
 
+Table* 
+error_return(memory::Root* mem_root_save,Session * session, Table* table)
+{
+  session->mem_root= mem_root_save;
+  table->free_tmp_table(session);
+  return NULL;
+}
+
 Table *
 create_tmp_table(Session *session,Tmp_Table_Param *param,List<Item> &fields,
 		 order_st *group, bool distinct, bool save_sum_fields,
@@ -2535,7 +2544,7 @@
                              false,
                              param->convert_blob_length);
 	  if (!new_field)
-	    goto err;					// Should be OOM
+	    return error_return(mem_root_save,session,table);					// Should be OOM
 	  tmp_from_field++;
 	  reclength+=new_field->pack_length();
 	  if (new_field->flags & BLOB_FLAG)
@@ -2589,7 +2598,7 @@
       if (!new_field)
       {
 	if (session->is_fatal_error)
-	  goto err;				// Got OOM
+	    return error_return(mem_root_save,session,table);				
 	continue;				// Some kindf of const item
       }
       if (type == Item::SUM_FUNC_ITEM)
@@ -2651,7 +2660,8 @@
     table->cursor= share->db_type()->getCursor(*share, &table->mem_root);
   }
   if (! table->cursor)
-    goto err;
+	    return error_return(mem_root_save,session,table);				
+   
 
 
   if (! using_unique_constraint)
@@ -2681,8 +2691,8 @@
     uint32_t alloc_length=ALIGN_SIZE(reclength+MI_UNIQUE_HASH_LENGTH+1);
     share->rec_buff_length= alloc_length;
     if (!(table->record[0]= (unsigned char*)
-                            alloc_root(&table->mem_root, alloc_length*3)))
-      goto err;
+                            alloc_root(&table->mem_root, alloc_length*3)))  
+	    return error_return(mem_root_save,session,table);				
     table->record[1]= table->record[0]+alloc_length;
     share->default_values= table->record[1]+alloc_length;
   }
@@ -2852,7 +2862,8 @@
                                                      test(maybe_null),
                                                      field->null_ptr,
                                                      field->null_bit)))
-	  goto err;
+	    return error_return(mem_root_save,session,table);				
+	  
 	if (maybe_null)
 	{
 	  /*
@@ -2901,7 +2912,8 @@
     if (!(key_part_info= (KEY_PART_INFO*)
           alloc_root(&table->mem_root,
                      keyinfo->key_parts * sizeof(KEY_PART_INFO))))
-      goto err;
+	    return error_return(mem_root_save,session,table);				
+      
     memset(key_part_info, 0, keyinfo->key_parts * sizeof(KEY_PART_INFO));
     table->key_info=keyinfo;
     keyinfo->key_part=key_part_info;
@@ -2929,8 +2941,8 @@
                                                 NULL,
                                                 table->s,
                                                 &my_charset_bin);
-      if (!key_part_info->field)
-        goto err;
+      if (!key_part_info->field) 
+	    	return error_return(mem_root_save,session,table);				
       key_part_info->field->init(table);
       key_part_info->key_type= 1; /* binary comparison */
       key_part_info->type=    HA_KEYTYPE_BINARY;
@@ -2971,25 +2983,24 @@
   }
 
   if (session->is_fatal_error)				// If end of memory
-    goto err;
+	    return error_return(mem_root_save,session,table);				
+    
   share->db_record_offset= 1;
   if (share->db_type() == myisam_engine)
   {
     if (table->create_myisam_tmp_table(param->keyinfo, param->start_recinfo,
 				       &param->recinfo, select_options))
-      goto err;
+	    return error_return(mem_root_save,session,table);				
+      
   }
   if (table->open_tmp_table())
-    goto err;
+	    return error_return(mem_root_save,session,table);				
+    
 
   session->mem_root= mem_root_save;
 
   return(table);
 
-err:
-  session->mem_root= mem_root_save;
-  table->free_tmp_table(session);
-  return NULL;
 }
 
 /****************************************************************************/