maria-developers team mailing list archive

[Bo Thorsen <bo@xxxxxxxxxxxx>]

The attached patch reduces the warnings in a 32 bit build with Visual 
Studio 2008 to a set in the flex/bison generated code. I'll handle those 
later.

I'll add comments on some of the changes here.

The libmysqld.def file: The description line isn't supported in the more 
recent compilers.

In fsp0fsp.c, there is an explicit cast to ullint. In this case, the 
code does what's intended, and the compiler warning is one of the "are 
you sure this is right" warnings. C4334 gives a warning if you make a
1u << 10 and store that in a 64 bit variable, because you could have 
meant 1i64 << 10.

The change in i_s.cc is the one I'm most worried about. It used to store 
the unsigned long long in a double. The change I did can be wrong. But 
even if it's not, I'm worried what happens when this runs on an existing 
set of tables.

We had a very long discussion about the cast in row0sel.c. The 
conclusion was that

* auto_increment on doubles or floats is a very odd case
* there is a compiler bug in Visual Studio for very large values (above
  around 2^53 it converts doubles wrong to uint64

I'd like to separate this discussion from this patch, though, and submit 
a bug report on the innodb code on mysql.

I assume there will be something here I should change, but if parts of 
the patch are ready for pushing, let me know that.

Bo Thorsen.
Monty Program AB.

--

MariaDB: MySQL replacement
Community developed. Feature enhanced. Backward compatible.

=== modified file 'libmysqld/libmysqld.def'
--- libmysqld/libmysqld.def	2010-01-15 15:27:55 +0000
+++ libmysqld/libmysqld.def	2010-07-07 14:24:17 +0000
@@ -1,5 +1,4 @@
 LIBRARY		LIBMYSQLD
-DESCRIPTION	'MySQL 5.1 Embedded Server Library'
 VERSION		5.1
 EXPORTS
 	mysql_thread_end

=== modified file 'storage/xtradb/buf/buf0flu.c'
--- storage/xtradb/buf/buf0flu.c	2010-04-28 14:35:00 +0000
+++ storage/xtradb/buf/buf0flu.c	2010-07-07 12:59:57 +0000
@@ -1050,7 +1050,7 @@
 					BUF_FLUSH_LIST; if BUF_FLUSH_LIST,
 					then the caller must not own any
 					latches on pages */
-	ulint		min_n,		/*!< in: wished minimum mumber of blocks
+	ib_uint64_t		min_n,		/*!< in: wished minimum mumber of blocks
 					flushed (it is not guaranteed that the
 					actual number is that big, though) */
 	ib_uint64_t	lsn_limit)	/*!< in the case BUF_FLUSH_LIST all

=== modified file 'storage/xtradb/buf/buf0lru.c'
--- storage/xtradb/buf/buf0lru.c	2010-01-06 12:00:14 +0000
+++ storage/xtradb/buf/buf0lru.c	2010-07-07 13:14:10 +0000
@@ -2074,7 +2074,8 @@
 buf_LRU_file_dump(void)
 /*===================*/
 {
-	os_file_t	dump_file = -1;
+	os_file_t	dump_file;
+	ibool		dump_file_open = 0;
 	ibool		success;
 	byte*		buffer_base = NULL;
 	byte*		buffer = NULL;
@@ -2103,8 +2104,8 @@
 	}
 
 	dump_file = os_file_create(LRU_DUMP_FILE, OS_FILE_OVERWRITE,
-				OS_FILE_NORMAL, OS_DATA_FILE, &success);
-	if (!success) {
+				OS_FILE_NORMAL, OS_DATA_FILE, &dump_file_open);
+	if (!dump_file_open) {
 		os_file_get_last_error(TRUE);
 		fprintf(stderr,
 			" InnoDB: cannot open %s\n", LRU_DUMP_FILE);
@@ -2164,7 +2165,7 @@
 
 	ret = TRUE;
 end:
-	if (dump_file != -1)
+	if (dump_file_open)
 		os_file_close(dump_file);
 	if (buffer_base)
 		ut_free(buffer_base);
@@ -2178,7 +2179,8 @@
 buf_LRU_file_restore(void)
 /*======================*/
 {
-	os_file_t	dump_file = -1;
+	os_file_t	dump_file;
+	ibool		dump_file_open = 0;
 	ibool		success;
 	byte*		buffer_base = NULL;
 	byte*		buffer = NULL;
@@ -2198,8 +2200,8 @@
 	}
 
 	dump_file = os_file_create_simple_no_error_handling(
-		LRU_DUMP_FILE, OS_FILE_OPEN, OS_FILE_READ_ONLY, &success);
-	if (!success) {
+		LRU_DUMP_FILE, OS_FILE_OPEN, OS_FILE_READ_ONLY, &dump_file_open);
+	if (!dump_file_open) {
 		os_file_get_last_error(TRUE);
 		fprintf(stderr,
 			" InnoDB: cannot open %s\n", LRU_DUMP_FILE);
@@ -2269,7 +2271,7 @@
 		" (requested: %lu, read: %lu)\n", req, reads);
 	ret = TRUE;
 end:
-	if (dump_file != -1)
+	if (dump_file_open)
 		os_file_close(dump_file);
 	if (buffer_base)
 		ut_free(buffer_base);

=== modified file 'storage/xtradb/fil/fil0fil.c'
--- storage/xtradb/fil/fil0fil.c	2010-04-30 04:23:39 +0000
+++ storage/xtradb/fil/fil0fil.c	2010-07-07 13:13:01 +0000
@@ -3023,7 +3023,8 @@
 		dulint		new_id[31];
 		ulint		root_page[31];
 		ulint		n_index;
-		os_file_t	info_file = -1;
+		os_file_t	info_file;
+		ibool		info_file_open = 0;
 		char*		info_file_path;
 		ulint	i;
 		int		len;
@@ -3080,8 +3081,8 @@
 		info_file_path[len - 1] = 'p';
 
 		info_file = os_file_create_simple_no_error_handling(
-				info_file_path, OS_FILE_OPEN, OS_FILE_READ_ONLY, &success);
-		if (!success) {
+				info_file_path, OS_FILE_OPEN, OS_FILE_READ_ONLY, &info_file_open);
+		if (!info_file_open) {
 			fprintf(stderr, "InnoDB: cannot open %s\n", info_file_path);
 			goto skip_info;
 		}
@@ -3109,7 +3110,7 @@
 		}
 
 skip_info:
-		if (info_file != -1)
+		if (info_file_open)
 			os_file_close(info_file);
 
 		/*

=== modified file 'storage/xtradb/fsp/fsp0fsp.c'
--- storage/xtradb/fsp/fsp0fsp.c	2010-04-28 19:29:45 +0000
+++ storage/xtradb/fsp/fsp0fsp.c	2010-07-07 13:25:18 +0000
@@ -3184,13 +3184,14 @@
 		return(0);
 	}
 
+	// The ullint cast on FSP_EXTENT_SIZE removes VS warning C4334
 	if (!zip_size) {
 		return((ullint) (n_free - reserve)
-		       * FSP_EXTENT_SIZE
+		       * ((ullint) FSP_EXTENT_SIZE)
 		       * (UNIV_PAGE_SIZE / 1024));
 	} else {
 		return((ullint) (n_free - reserve)
-		       * FSP_EXTENT_SIZE
+		       * ((ullint) FSP_EXTENT_SIZE)
 		       * (zip_size / 1024));
 	}
 }

=== modified file 'storage/xtradb/handler/i_s.cc'
--- storage/xtradb/handler/i_s.cc	2010-04-28 14:35:00 +0000
+++ storage/xtradb/handler/i_s.cc	2010-07-07 12:18:50 +0000
@@ -2821,7 +2821,7 @@
 
 		field_store_string(i_s_table->field[0], buf);
 		field_store_string(i_s_table->field[1], ptr);
-		i_s_table->field[2]->store(table->stat_n_rows);
+		i_s_table->field[2]->store(table->stat_n_rows, true);
 		i_s_table->field[3]->store(table->stat_clustered_index_size);
 		i_s_table->field[4]->store(table->stat_sum_of_other_index_sizes);
 		i_s_table->field[5]->store(table->stat_modified_counter);

=== modified file 'storage/xtradb/include/buf0flu.h'
--- storage/xtradb/include/buf0flu.h	2009-09-07 10:22:53 +0000
+++ storage/xtradb/include/buf0flu.h	2010-07-07 13:00:01 +0000
@@ -82,7 +82,7 @@
 					BUF_FLUSH_LIST; if BUF_FLUSH_LIST,
 					then the caller must not own any
 					latches on pages */
-	ulint		min_n,		/*!< in: wished minimum mumber of blocks
+	ib_uint64_t		min_n,		/*!< in: wished minimum mumber of blocks
 					flushed (it is not guaranteed that the
 					actual number is that big, though) */
 	ib_uint64_t	lsn_limit);	/*!< in the case BUF_FLUSH_LIST all

=== modified file 'storage/xtradb/row/row0sel.c'
--- storage/xtradb/row/row0sel.c	2010-04-28 14:35:00 +0000
+++ storage/xtradb/row/row0sel.c	2010-07-07 13:32:33 +0000
@@ -4650,12 +4650,12 @@
 
 	case DATA_FLOAT:
 		ut_a(len == sizeof(float));
-		value = mach_float_read(data);
+		value = (ib_uint64_t)mach_float_read(data);
 		break;
 
 	case DATA_DOUBLE:
 		ut_a(len == sizeof(double));
-		value = mach_double_read(data);
+		value = (ib_uint64_t)mach_double_read(data);
 		break;
 
 	default: