[sr-dev] ndb_redis compilation issues
Vicente Hernando
vhernando at systemonenoc.com
Fri May 1 01:47:34 CEST 2015
Hi Alex
On 04/30/2015 08:46 PM, Alex Balashov wrote:
> Hello,
>
> I have a problem with building hiredis + ndb_redis. The problem is
> with hiredis rather than ndb_redis directly, but I am not sure how to
> best handle it and would be grateful for any advice.
>
> The problem is that somewhere with the recent Hiredis 0.13.0
> release[1], they came to use the following nested struct in the
> hiredis.h header:
>
> ---
> /* Context for a connection to Redis */
> typedef struct redisContext {
> ...
> struct {
> char *path;
> } unix; /* line 158 */
> ...
> } redisContext;
> ---
>
> It appears the GNU compilers do not like the use of 'unix' as an
> identifier:
>
> ---
> [root at proxy ndb_redis]# make
> CC (gcc) [M ndb_redis.so] ndb_redis_mod.o
> In file included from redis_client.h:27,
> from ndb_redis_mod.c:38:
> /usr/local/include/hiredis/hiredis.h:158: error: expected identifier
> or ‘(’ before numeric constant
> make: *** [ndb_redis_mod.o] Error 1
> ---
>
> Indeed, a quick test against a normal gcc installation on CentOS 6.6
> reveals that 'unix' is a defined constant:
>
> ---
> [root at proxy ~]# echo -e "#include <stdio.h>\n#include <stdlib.h>\nint
> main() { printf(\"%d\\\n\", unix); return 1; }" > ns.c
> [root at proxy ~]# make ns
> cc ns.c -o ns
> [root at proxy ~]# ./ns
> 1
> ---
>
> The authors of hiredis themselves note this problem in a recent commit:
>
> ---
> commit d8145d79ce715054980938c751067ebaa541573c
> Author: Jan-Erik Rediger <janerik at fnordig.de>
> Date: Thu Apr 16 22:51:32 2015 +0200
>
> Always compile with C99 standard.
>
> Turns out: gnu9x defines `unix` to 1, making it unusable as a
> variable
> name.
> ---
>
> So, their solution is to compile with -std=c99, which works fine.
>
> The problem is that a) ndb_redis isn't compiled with -std=c99 and b)
> my superficial effort at making it compile that way did not bear fruit:
>
https://gcc.gnu.org/onlinedocs/gcc-4.4.6/gcc/Alternate-Keywords.html#Alternate-Keywords
All asm keywords in some kamailio files should be changed to __asm__
Also we should add something like this to source code in ndb_redis module:
#if __STDC_VERSION__ >= 199901L
#define _XOPEN_SOURCE 600
#else
#define _XOPEN_SOURCE 500
#endif /* __STDC_VERSION__ */
and still a lot of errors persist with other unset macros like
__USE_MISC or __USE_GNU.
Compiling with -std=c99 option looks weird for kamailio.
> ---
> [root at rproxy01 ndb_redis]# make MOD_CFLAGS='-fPIC -DPIC $(CFLAGS)
> -std=c99'
> CC (gcc) [M ndb_redis.so] ndb_redis_mod.o
> In file included from ../../parser/../mem/shm_mem.h:47,
> from ../../parser/../ut.h:64,
> from ../../parser/../ip_addr.h:50,
> from ../../parser/msg_parser.h:61,
> from ../../sr_module.h:62,
> from ndb_redis_mod.c:32:
> /usr/include/sys/ipc.h:25:3: warning: #warning "Files using this
> header must be compiled with _SVID_SOURCE or _XOPEN_SOURCE"
> In file included from ../../parser/../mem/../atomic/atomic_native.h:53,
> from ../../parser/../mem/../futexlock.h:44,
> from ../../parser/../mem/../lock_ops.h:85,
> from ../../parser/../mem/shm_mem.h:75,
> from ../../parser/../ut.h:64,
> from ../../parser/../ip_addr.h:50,
> from ../../parser/msg_parser.h:61,
> from ../../sr_module.h:62,
> from ndb_redis_mod.c:32:
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_inc_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:226: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:226: error: (Each
> undeclared identifier is reported only once
> ../../parser/../mem/../atomic/atomic_x86.h:226: error: for each
> function it appears in.)
> ../../parser/../mem/../atomic/atomic_x86.h:226: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_dec_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:227: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:227: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_and_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:228: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:228: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_or_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:229: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:229: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_inc_and_test_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:230: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:230: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_dec_and_test_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:231: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:231: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_get_and_set_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:232: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:232: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_cmpxchg_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:233: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:233: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_add_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:234: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:234: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_inc_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:236: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:236: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_dec_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:237: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:237: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_and_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:238: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:238: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function ‘atomic_or_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:239: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:239: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_inc_and_test_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:240: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:240: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_dec_and_test_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:241: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:241: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_get_and_set_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:242: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:242: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_cmpxchg_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:243: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:243: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘atomic_add_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:244: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:244: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘mb_atomic_set_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:299: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:299: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘mb_atomic_set_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:312: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:312: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘mb_atomic_get_int’:
> ../../parser/../mem/../atomic/atomic_x86.h:331: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:331: error: expected ‘;’
> before ‘volatile’
> ../../parser/../mem/../atomic/atomic_x86.h: In function
> ‘mb_atomic_get_long’:
> ../../parser/../mem/../atomic/atomic_x86.h:342: error: ‘asm’
> undeclared (first use in this function)
> ../../parser/../mem/../atomic/atomic_x86.h:342: error: expected ‘;’
> before ‘volatile’
> In file included from ../../parser/../mem/../lock_ops.h:85,
> from ../../parser/../mem/shm_mem.h:75,
> from ../../parser/../ut.h:64,
> from ../../parser/../ip_addr.h:50,
> from ../../parser/msg_parser.h:61,
> from ../../sr_module.h:62,
> from ndb_redis_mod.c:32:
> ../../parser/../mem/../futexlock.h: In function ‘futex_get’:
> ../../parser/../mem/../futexlock.h:110: warning: implicit declaration
> of function ‘syscall’
> ndb_redis_mod.c: At top level:
> ../../parser/parse_param.h:156: warning: inline function ‘parse_param’
> declared but never defined
> make: *** [ndb_redis_mod.o] Error 1
> ---
>
> There are two separate problems here:
>
> 1) My invocation of 'make' above may betray an insufficient
> understanding the Makefile chain & build process. Is there something
> I'm doing wrong there?
>
> 2) In choosing to work around the issue this way, hiredis has imposed
> the requirement on GNU C users to compile any applications utilising
> hiredis with C99 themselves. This doesn't seem like a good solution.
> Is there a more 'elegant' fix here? It seems to me they ought to
> rename that 'unix' struct member.
>
> The blame-worthy hiredis commit seems to be this one:
>
> ---
> commit d9e0b0f6abfbb8918f73607bdfcc707d0df3fd41
> Author: Jan-Erik Rediger <janerik at fnordig.de>
> Date: Thu Apr 16 19:28:12 2015 +0200
>
> Implement a reconnect method for the client context
>
> Originally implemented by @abedra as part of #306.
>
> In case a write or read times out, we force an error state,
> because we
> can't guarantuee that the next read will get the right data.
> Instead we need to reconnect to have a clean-state connection,
> which is
> now easily possible with this method.
>
> i.e.
>
> /* Context for a connection to Redis */
> typedef struct redisContext {
> int err; /* Error flags, 0 when there is no error */
> @@ -136,6 +141,20 @@ typedef struct redisContext {
> int flags;
> char *obuf; /* Write buffer */
> redisReader *reader; /* Protocol reader */
> +
> + enum redisConnectionType connection_type;
> + struct timeval *timeout;
> +
> + struct {
> + char *host;
> + char *source_addr;
> + int port;
> + } tcp;
> +
> + struct {
> + char *path;
> + } unix;
> +
> } redisContext;
> ---
>
> Reverting to the parent commit,
> b872919463fbc04c3a8fde113cb9ae89dfcb3859, seems to fix the problem.
> That's my workaround for now.
>
I vote for that too.
May changing every unix occurrence in hiredis code for the patch:
grep output:
$ grep -nH unix -r *
Binary file async.o matches
hiredis.c:605: c->unix.path = NULL;
hiredis.c:629: if (c->unix.path)
hiredis.c:630: free(c->unix.path);
hiredis.c:661: return redisContextConnectUnix(c, c->unix.path,
c->timeout);
hiredis.h:158: } unix;
net.c:425: if (c->unix.path != path)
net.c:426: c->unix.path = strdup(path);
test.c:33: } unix;
test.c:100: c = redisConnectUnix(config.unix.path);
test.c:103: redisContext *dummy_ctx =
redisConnectUnix(config.unix.path);
test.c:484: config.unix.path = "foo";
test.c:738: .unix = {
test.c:759: cfg.unix.path = argv[0];
test.c:785: printf("\nTesting against Unix socket connection
(%s):\n", cfg.unix.path);
test.c:793: printf("\nTesting against inherited fd (%s):\n",
cfg.unix.path);
Regards,
Vicente.
> Thanks!
>
> -- Alex
>
> [1] Around 16 Apr 2015.
>
More information about the sr-dev
mailing list