Bug 227244 - databases/mariadb102-server: Fix build with clang 6
Summary: databases/mariadb102-server: Fix build with clang 6
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Bernard Spil
URL: https://jira.mariadb.org/browse/MDEV-...
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-03 08:26 UTC by Dimitry Andric
Modified: 2018-05-06 19:45 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (brnrd)


Attachments
Return 0 instead of NULL to fix build with clang 6 and higher (2.63 KB, patch)
2018-04-03 08:26 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric freebsd_committer 2018-04-03 08:26:14 UTC
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.
Comment 1 Bernard Spil freebsd_committer 2018-04-03 17:34:41 UTC
Reported upstream as well. This is for 12-STABLE?
Comment 2 Dries Michiels 2018-04-04 19:44:03 UTC
Reproducible for 11 STABLE
Comment 3 xavier 2018-04-07 14:00:25 UTC
I confirm the problem with 11 STABLE
Comment 4 Dimitry Andric freebsd_committer 2018-05-05 10:59:14 UTC
Bernard, is it OK to commit this patch now, or is there an update from upstream?
Comment 5 Bernard Spil freebsd_committer 2018-05-06 12:21:24 UTC
(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?
Comment 6 Dimitry Andric freebsd_committer 2018-05-06 13:36:22 UTC
(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.
Comment 7 Bernard Spil freebsd_committer 2018-05-06 18:43:21 UTC
(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
Comment 8 Bernard Spil freebsd_committer 2018-05-06 19:03:27 UTC
Checked some more code of MariaDB and using 0 makes sense. Commit coming up after QA builds.
Comment 9 commit-hook freebsd_committer 2018-05-06 19:23:40 UTC
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
Comment 10 Dimitry Andric freebsd_committer 2018-05-06 19:38:52 UTC
(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... :)
Comment 11 commit-hook freebsd_committer 2018-05-06 19:44:03 UTC
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
Comment 12 Bernard Spil freebsd_committer 2018-05-06 19:44:54 UTC
Thanks Dimitry! Learned things again today. Committed to 10.0 too.