@miconda commented on this pull request.

@@ -2147,6 +2401,9 @@ static int child_init(int rank)

/* probe rtpengines only in first worker */ if(build_rtpp_socks(0, 1)) return -1; + + if (rtpengine_dtmf_event_sock.len > 0) + rtpengine_dtmf_events_loop(rank);

It seems to block the child process with rank 1 (PROC_SIPINIT), which should not be done, because it can create problems for other modules such as usrloc.

A new process has to be started -- in mod init, it has to be registered with:

``` register_procs(1); cfg_register_child(1); ```

And in child init created with fork_process(), something like:

``` if(rank == PROC_MAIN) { /* fork worker process */ newpid = fork_process(PROC_RPC, "RTPENGINE DTMF WORKER", 1); if(newpid < 0) { LM_ERR("failed to fork worker process %d\n", i); return -1; } else if(newpid == 0) { if(cfg_child_init()) return -1; /* child - this will loop forever */ rtpengine_dtmf_events_loop(); } else { /* parent process*/ } return 0; } ```

@joeygo commented on this pull request.

@@ -2147,6 +2401,9 @@ static int child_init(int rank)

/* probe rtpengines only in first worker */ if(build_rtpp_socks(0, 1)) return -1; + + if (rtpengine_dtmf_event_sock.len > 0) + rtpengine_dtmf_events_loop(rank);

Thanks for the example. I fixed the code.

@rfuchs commented on this pull request.

• s_port.len = rtpengine_dtmf_event_sock.s + rtpengine_dtmf_event_sock.len - s_port.s;

+ + if (s_port.len <= 0 || str2int(&s_port, &port) < 0 || port > 65535) { + LM_ERR("failed to initialize dtmf event listener because port is invalid %.*s\n", rtpengine_dtmf_event_sock.len, rtpengine_dtmf_event_sock.s); + return; + } + rtpengine_dtmf_event_sock.len -= s_port.len + 1; + trim(&rtpengine_dtmf_event_sock); + rtpengine_dtmf_event_sock.s[rtpengine_dtmf_event_sock.len] = '\0'; + + memset(&udp_addr, 0, sizeof(udp_addr)); + + if (rtpengine_dtmf_event_sock.s[0] == '[') { + udp_addr.sin6.sin6_family = AF_INET6; + udp_addr.sin6.sin6_port = htons(port); + ret = inet_pton(AF_INET, rtpengine_dtmf_event_sock.s, &udp_addr.sin6.sin6_addr);

Must be AF_INET6 here

• ret = inet_pton(AF_INET, rtpengine_dtmf_event_sock.s, &udp_addr.sin.sin_addr);
  

+ } + + if (ret != 1) { + LM_ERR("failed to initialize dtmf event listener because address could not be created for %s\n", rtpengine_dtmf_event_sock.s); + return; + } + + rtpengine_dtmf_event_fd = socket(udp_addr.s.sa_family, SOCK_DGRAM, 0); + + if(rtpengine_dtmf_event_fd < 0) { + LM_ERR("can't create socket\n"); + return; + } + + if (bind(rtpengine_dtmf_event_fd, (struct sockaddr*)&udp_addr.s, sizeof(udp_addr.s)) < 0) {

sizeof(udp_addr.s) is sizeof(struct sockaddr) which I think may be too small for INET6. sizeof(udp_addr) would be a better option

• }
  

+ + if (dtmf_event_rt == -1) { + LM_NOTICE("nothing to do - nobody is listening!\n"); + goto end; + } + + p = shm_malloc(ret + 1); + if (!p) { + LM_ERR("could not allocate %d for buffer %.*s\n", ret, ret, buffer); + goto end; + } + memcpy(p, buffer, ret); + p[ret] = '\0'; + + if (rtpengine_raise_dtmf_event(p, ret) < 0) {

I think this leaks the `p` buffer if the return value == 0

+end: + close(rtpengine_dtmf_event_fd); +} + +static int rtpengine_raise_dtmf_event(char *buffer, int len) { + srjson_doc_t jdoc; + srjson_t *it = NULL; + struct sip_msg *fmsg = NULL; + struct run_act_ctx ctx; + int rtb; + + LM_DBG("executing event_route[rtpengine:dtmf-event] (%d)\n", dtmf_event_rt); + LM_DBG("dispatching buffer: %s\n", buffer); + + srjson_InitDoc(&jdoc, NULL);

jdoc needs to be freed somewhere

@@ -261,7 +262,7 @@ static int add_rtpp_node_info(

void *ptrs, struct rtpp_node *crt_rtpp, struct rtpp_set *rtpp_list); static int rtpp_test_ping(struct rtpp_node *node);

-static void rtpengine_dtmf_events_loop(int rank); +static void rtpengine_dtmf_events_loop();

nit: for better standards compliance, be explicit with `(void)`

• }

+ + if(rank == PROC_MAIN) { + if(rtpengine_dtmf_event_sock.len > 0) { + LM_DBG("Register RTPENGINE DTMF WORKER %d\n", mypid); + /* fork worker process */ + mypid = fork_process(PROC_RPC, "RTPENGINE DTMF WORKER", 1); + if(mypid < 0) { + LM_ERR("failed to fork RTPENGINE DTMF WORKER process %d\n", mypid); + return -1; + } else if(mypid == 0) { + if(cfg_child_init()) + return -1; + /* this will loop forever */ + rtpengine_dtmf_events_loop(); + }

I suppose there should be an exit() at the end here? Although I'm not too familiar with how the Kamailio internals are handling this

@joeygo commented on this pull request.

@@ -261,7 +262,7 @@ static int add_rtpp_node_info(

void *ptrs, struct rtpp_node *crt_rtpp, struct rtpp_set *rtpp_list); static int rtpp_test_ping(struct rtpp_node *node);

-static void rtpengine_dtmf_events_loop(int rank); +static void rtpengine_dtmf_events_loop();

Fixed.