Created attachment 192159 [details] Return 0 instead of NULL to fix build with clang 6 and higher When building with clang 6, which defaults to -std=gnu++14 for C++ mode, databases/mariadb102-server fails to build, with the following errors: [ 48%] Building CXX object storage/connect/CMakeFiles/connect.dir/tabjson.cpp.o cd /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect && /usr/bin/c++ -DFORCE_INIT_OF_VARS -DGZ_SUPPORT -DHAVE_CONFIG_H -DHUGE_SUPPORT -DLIBXML2_SUPPORT -DLINUX -DMARIADB -DMYSQL_DYNAMIC_PLUGIN -DNOCRYPT -DODBC_SUPPORT -DUBUNTU -DUNIX -DVCT_SUPPORT -DXMAP -DZIP_SUPPORT -Dconnect_EXPORTS -I/wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/include -I/wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/sql -I/wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/pcre -I/usr/local/include -I/usr/local/include/libxml2 -O2 -pipe -march=haswell -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -isystem /usr/local/include -Wl,-z,relro,-z,now -fstack-protector --param=ssp-buffer-size=4 -DWITH_INNODB_DISALLOW_WRITES -fno-rtti -Wall -Wmissing-declarations -Wno-unused-function -Wno-unused-variable -Wno-unused-value -Wno-parentheses -Wno-strict-aliasing -Wno-implicit-fallthrough -fpermissive -fexceptions -fPIC -O2 -pipe -march=haswell -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -isystem /usr/local/include -D_FORTIFY_SOURCE=2 -DDBUG_OFF -fPIC -o CMakeFiles/connect.dir/tabjson.cpp.o -c /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp c++: warning: -Wl,-z,relro,-z,now: 'linker' input unused [-Wunused-command-line-argument] /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:198:10: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:246:11: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:253:12: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:272:12: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:279:12: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:288:12: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ /wrkdirs/usr/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp:307:11: error: cannot initialize return object of type 'int' with an rvalue of type 'nullptr_t' return NULL; ^~~~ /usr/include/sys/_null.h:35:14: note: expanded from macro 'NULL' #define NULL nullptr ^~~~~~~ 7 errors generated. In this case, NULL is not suitable as a return value for a function returning int. (Before C++11, NULL was usually defined as plain zero, which is why this bad code used to compile.) Fix it by returning integer zero instead, like in most other places in the same function.
Reported upstream as well. This is for 12-STABLE?
Reproducible for 11 STABLE
I confirm the problem with 11 STABLE
Bernard, is it OK to commit this patch now, or is there an update from upstream?
(In reply to Dimitry Andric from comment #4) Just looked a bit more a the code and the return uses various values: NULL, 0 and 0ULL. For a method with type int I think it's weird to return an unsigned long long, or am I mistaken?
(In reply to Bernard Spil from comment #5) > (In reply to Dimitry Andric from comment #4) > Just looked a bit more a the code and the return uses various values: NULL, > 0 and 0ULL. For a method with type int I think it's weird to return an > unsigned long long, or am I mistaken? I don't see those instances in tabjson.cpp, which functions are you talking about? There is no function that returns 0ULL: $ grep 'return 0' /wrkdirs/share/dim/ports/databases/mariadb102-server/work/mariadb-10.2.14/storage/connect/tabjson.cpp return 0; return 0; return 0; return 0; return 0; I think upstream has just made a number of copy/paste errors, because some functions in this file are supposed to return pointers (e.g. PQRYRES or PTDB types), and such functions return NULL in case of failure. Other functions return int, and are likely returning zero to indicate failure, but probably some lines were copied from the other functions which return pointers. If the C++ mode is before C++11, NULL is usually defined as a literal '0', so then you won't even notice the difference between NULL and 0. But from C++11 onwards, NULL is usually defined as the keyword 'nullptr', which is not implicitly convertible to an integer.
(In reply to Dimitry Andric from comment #6) Hi Dimitry, I was looking at https://github.com/MariaDB/server/blob/10.3/storage/connect/tabjson.cpp
Checked some more code of MariaDB and using 0 makes sense. Commit coming up after QA builds.
A commit references this bug: Author: brnrd Date: Sun May 6 19:23:26 UTC 2018 New revision: 469246 URL: https://svnweb.freebsd.org/changeset/ports/469246 Log: databases/mariadb102-server: Fix builds - Fix build on aarch64 [1] - Fix build with clang6 (HEAD, 11-STABLE) [2] PR: 227628 [1], 227244 [2] Submitted by: Naram Qashat <cyberbotx cyberbotx com> [1] Submitted by: dim Changes: head/databases/mariadb102-server/Makefile head/databases/mariadb102-server/files/patch-MDEV-15768 head/databases/mariadb102-server/files/patch-MDEV-15961
(In reply to Bernard Spil from comment #7) > https://github.com/MariaDB/server/blob/10.3/storage/connect/tabjson.cpp It looks like quite a lot of editing and merging was done on that file, and I think this commit: https://github.com/MariaDB/server/commit/273233119c575d34d1c7b6cf649150f36d200242 changed the signature of the following function: PQRYRES JSONColumns(PGLOBAL g, PCSZ db, PCSZ dsn, PTOS topt, bool info) to: int JSONDISC::GetColumns(PGLOBAL g, PCSZ db, PCSZ dsn, PTOS topt) So before the commit, the function *did* return a pointer, and NULL was an appropriate return value. After the commit, however, it should start returning plain integers instead. It looks like lots of the changes in this area were done pretty sloppily, or in haste, as large parts of code were commented out, then uncommented again in later commits, probably to work around high priority bugs... :)
A commit references this bug: Author: brnrd Date: Sun May 6 19:43:46 UTC 2018 New revision: 469249 URL: https://svnweb.freebsd.org/changeset/ports/469249 Log: databases/mariadb100-server: Fix build with clang 6 PR: 227244 Submitted by: dim Changes: head/databases/mariadb100-server/files/patch-MDEV-15768
Thanks Dimitry! Learned things again today. Committed to 10.0 too.