Bug #59905 Plugin boolean variables don't behave well on unsigned-char machines

Description:
mysql 5.5.8 fails its regression tests when built on a machine that considers C's "char" type to be unsigned char, such as PPC.  The failures look like this:

sys_vars.rpl_semi_sync_master_enabled_basic [ fail ]
        Test ended at 2011-01-31 17:18:49
CURRENT_TEST: sys_vars.rpl_semi_sync_master_enabled_basic
--- /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2010-12-03 20:58:25.000000000 +0300
+++ /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.reject	2011-01-31 20:18:49.313402710 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_master_enabled;
 @@global.rpl_semi_sync_master_enabled
--1
+255
 select @@session.rpl_semi_sync_master_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_master_enabled';
mysqltest: Result content mismatch

sys_vars.rpl_semi_sync_slave_enabled_basic [ fail ]
        Test ended at 2011-01-31 17:18:50
CURRENT_TEST: sys_vars.rpl_semi_sync_slave_enabled_basic
--- /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2010-12-03 20:58:26.000000000 +0300
+++ /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.reject	2011-01-31 20:18:50.555904753 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+255
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';
mysqltest: Result content mismatch

Observed on Fedora rawhide and RHEL-6, but I don't believe the specific OS matters, only the C compiler's ideas about "char".

How to repeat:
1. Build on PPC, or another architecture where char is unsigned by default.  It'd likely also work to use a compiler switch that changes the default signedness of char, but I didn't try that.

2. Run the standard regression tests.

Suggested fix:
The minimum fix would be to paper over the problem by forcing bool variables to print signed everywhere, for example with this horrid kluge:

diff -Naur mysql-5.5.8.orig/sql/item_func.cc mysql-5.5.8/sql/item_func.cc
--- mysql-5.5.8.orig/sql/item_func.cc	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/sql/item_func.cc	2011-02-01 19:01:08.536298538 -0500
@@ -5442,7 +5442,7 @@
     case SHOW_LONGLONG: get_sys_var_safe (ulonglong);
     case SHOW_HA_ROWS:  get_sys_var_safe (ha_rows);
     case SHOW_BOOL:     get_sys_var_safe (bool);
-    case SHOW_MY_BOOL:  get_sys_var_safe (my_bool);
+    case SHOW_MY_BOOL:  get_sys_var_safe (signed char);
     case SHOW_DOUBLE:
       {
         double dval= val_real();

I don't much care for that though, because I think the behavior shown in the regression tests is wrong/insane anyway.  When you set a boolean variable to 1, it does not meet the principle of least astonishment for it to print out as -1.  I think the real bug is that somebody stuck a minus sign into check_func_bool() --- what the heck is the reasoning for that?  I'm planning to use the following patch instead:

diff -Naur mysql-5.5.8.orig/sql/sql_plugin.cc mysql-5.5.8/sql/sql_plugin.cc
--- mysql-5.5.8.orig/sql/sql_plugin.cc	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/sql/sql_plugin.cc	2011-02-01 20:34:10.218305349 -0500
@@ -2024,7 +2024,7 @@
       goto err;
     result= (int) tmp;
   }
-  *(my_bool *) save= -result;
+  *(my_bool *) save= result ? true : false;
   return 0;
 err:
   return 1;
diff -Naur mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result
--- mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2010-12-03 12:58:25.000000000 -0500
+++ mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2011-02-01 21:53:20.006302245 -0500
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_master_enabled;
 @@global.rpl_semi_sync_master_enabled
--1
+1
 select @@session.rpl_semi_sync_master_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_master_enabled';
diff -Naur mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result
--- mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2011-02-01 21:53:59.689249491 -0500
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+1
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';

Note that these two regression tests are the only ones in the "standard" suite that show any change in results from this, but there might be some more elsewhere.

Thank you for the report.

Verified as described using x86_64 architecture and gcc option -funsigned-char

Also affects 5.1.

The minus in sql_plugin.cc looks like a residual left over. At first, the backing type of boolean variables was int (which in itself was a bug), but later got converted to my_bool.

This also affects the ARM architecture, and can be seen causing spurious failures in the test suite on Ubuntu builds for armel and armhf:

https://launchpadlibrarian.net/110613104/buildlog_ubuntu-quantal-armhf.mysql-5.5_5.5.25a-0...

sys_vars.rpl_semi_sync_slave_enabled_basic [ fail ]
        Test ended at 2012-07-20 01:24:11

CURRENT_TEST: sys_vars.rpl_semi_sync_slave_enabled_basic
--- /build/buildd/mysql-5.5-5.5.25a/builddir/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2012-07-20 03:26:15.000000000 +0300
+++ /build/buildd/mysql-5.5-5.5.25a/builddir/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.reject	2012-07-20 04:24:11.000000000 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+255
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';

Given that there are real, viable ARM servers on the market now, this seems rather important to get solved.

Tracking in Ubuntu here:

https://bugs.launchpad.net/mysql-server/+bug/1080582

Noted in 5.6.13, 5.7.2 changelogs.

Boolean plugin system variables did not behave well on machines where
char is unsigned; some code attempted to assign a negative value to
these.