awslabs / aws-elasticache-cluster-client-libmemcached

Libmemcached library support for Amazon ElastiCache Memcached Cluster for PHP. The client is available at https://github.com/awslabs/aws-elasticache-cluster-client-memcached-for-php.
Other
26 stars 24 forks source link

Fix compile error with g++ -std=c++0x #15

Closed yujinis closed 4 years ago

yujinis commented 5 years ago

When I tried to built this on ALAMI2 with gcc-7.3.1-5.amzn2.0.2.x86_64, it failed as follows.

make -j3  all-am
make[1]: Entering directory `/home/ec2-user/aws-elasticache-cluster-client-libmemcached-1.0.18/BUILD'
  CXX      clients/memflush.o
In file included from ../libmemcached-1.0/memcached.h:113:0,
                 from ../clients/memflush.cc:21:
../libmemcached-1.0/server.h:118:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
 const bool has_memcached_server_ipaddress(const memcached_server_st *self);
 ^~~~~
../libmemcached-1.0/server.h:121:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
 const bool has_memcached_instance_ipaddress(const memcached_instance_st *self);
 ^~~~~
../clients/memflush.cc: In function ‘int main(int, char**)’:
../clients/memflush.cc:42:22: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
   if (opt_servers == false)
                      ^~~~~
../clients/memflush.cc:51:24: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
     if (opt_servers == false)
                        ^~~~~
make[1]: *** [clients/memflush.o] Error 1
make[1]: Leaving directory `/home/ec2-user/aws-elasticache-cluster-client-libmemcached-1.0.18/BUILD'
make: *** [all] Error 2

Then, I found opt_servers variable is a pointer for char type and realized it should NOT be compared with boolean. I noticed in Makefile that -std=c++0x was specified and probably this could find the error.

NiniGeek commented 5 years ago

Same problem when I build it on amazon linux 2. This fixed my error. Thanks.

yujinis commented 5 years ago

@NiniGeek Most probably this won't change the logic but for safety reason, this should be merged before it is applied broadly.

yujinis commented 5 years ago

Let me add my findings. This issue could happen according to the package version c++ command is owned by. Note that successful compile did not generate any output.

My test code

$ cat a.cc
#include <stdio.h>
#include <stdbool.h>

static char *opt_servers= NULL;

int main() {
    if(opt_servers == false){
        printf("false\n");
    }else{
        printf("true\n");
    }
}

ALAMI2

$ uname -a
Linux ip-172-31-19-59.ap-northeast-1.compute.internal 4.14.77-81.59.amzn2.x86_64 #1 SMP Mon Nov 12 21:32:48 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
$ which c++
/usr/bin/c++
$ ls -l /usr/bin/c++
-rwxr-xr-x 4 root root 1030976  7月 23  2018 /usr/bin/c++
$ rpm -qf /usr/bin/c++
gcc-c++-7.3.1-5.amzn2.0.2.x86_64
$ c++ a.cc -std=c++98
$ c++ a.cc -std=c++11
a.cc: In function ‘int main()’:
a.cc:7:20: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
  if(opt_servers == false){
                    ^~~~~
$ c++ a.cc -std=c++1y      
a.cc: In function ‘int main()’:
a.cc:7:20: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
  if(opt_servers == false){
                    ^~~~~

ALAMI

$ uname -a
Linux ip-172-31-24-165 4.1.17-22.30.amzn1.x86_64 #1 SMP Fri Feb 5 23:44:22 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ which c++
/usr/bin/c++
$ ls -l /usr/bin/c++
lrwxrwxrwx 1 root root 21 Feb 24 06:13 /usr/bin/c++ -> /etc/alternatives/c++
$ ls -l /etc/alternatives/c++
lrwxrwxrwx 1 root root 14 Feb 24 06:13 /etc/alternatives/c++ -> /usr/bin/g++48
$ ls -l /usr/bin/g++48
-rwxr-xr-x 4 root root 760192 May  8  2018 /usr/bin/g++48
$ rpm -qf /usr/bin/g++48
gcc48-c++-4.8.5-28.142.amzn1.x86_64
$ c++ a.cc -std=c++98
$ c++ a.cc -std=c++11
$ c++ a.cc -std=c++1y
ajayrockrock commented 5 years ago

@yulazari Can we get this merged in? This is breaking my docker builds when you use the default php:latest container as it's been upgraded to debian buster.

QuChen88 commented 4 years ago

Thanks for fixing this. Merged