@oej commented on this pull request.
You're doing good. Just a few small nits after reading it again.
> @@ -883,6 +883,57 @@ if(dns_query("test.com", "xyz"))
</section>
+ <section id="ipops.f.ptr_query">
+ <title>
+ <function moreinfo="none">ptr_query(ip, pvid)</function>
+ </title>
+
+ <para>
+ Store the hostname that correspond to ip
s/ip/an IP address/
Also document if you support both IPv4 and IPv6 clearly. Thanks!
> @@ -883,6 +883,57 @@ if(dns_query("test.com", "xyz"))
</section>
+ <section id="ipops.f.ptr_query">
+ <title>
+ <function moreinfo="none">ptr_query(ip, pvid)</function>
+ </title>
+
+ <para>
+ Store the hostname that correspond to ip
+ in a config variable $ptrquery(pvid=>hostname).
You mean a "pseudo variable" not a config variable.
> + <title>
+ <function moreinfo="none">ptr_query(ip, pvid)</function>
+ </title>
+
+ <para>
+ Store the hostname that correspond to ip
+ in a config variable $ptrquery(pvid=>hostname).
+ </para>
+
+ <para>Parameters:</para>
+
+ <itemizedlist>
+ <listitem>
+ <para>
+ <emphasis>ip</emphasis> - string or pseudo-variable
containing the ip.
+ The resulting IP addresses from DNS query are
compared with ipaddr.
In line 901 you just say "ip" and here "ipaddr". I suggest you write clear text
on both cases. Like "containing the IP address" and "compared with the IP
address"
> + Store the hostname that correspond to ip
+ in a config variable $ptrquery(pvid=>hostname).
+ </para>
+
+ <para>Parameters:</para>
+
+ <itemizedlist>
+ <listitem>
+ <para>
+ <emphasis>ip</emphasis> - string or pseudo-variable
containing the ip.
+ The resulting IP addresses from DNS query are
compared with ipaddr.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <emphasis>pvid</emphasis> - container id for script variable.
I don't think we call a pseudo variable a "container" anywhere. If so, I
consider it a bug :-)
> @@ -418,6 +413,91 @@ int dns_update_pv(str *hostname, str *name)
return 1;
}
+/*
+*
Add a comment here
> @@ -24,13 +24,46 @@
#define _IPOPS_PV_H_
#include "../../core/pvar.h"
+#define PV_DNS_ADDR 64
+#define PV_DNS_RECS 32
+#define SR_DNS_PVIDX 1
+
+typedef struct _sr_dns_record
+{
+ int type;
+ char addr[PV_DNS_ADDR];
+} sr_dns_record_t;
+typedef struct _sr_dns_item
Empty line missing
> +/**
+ *
+ */
+static int w_ptr_query(sip_msg_t *msg, char *ip, char *pv_name)
+{
+ str ip_address;
+ str name;
+
+ if(msg == NULL) {
+ LM_ERR("received null msg\n");
+ return -1;
+ }
+
+ if(fixup_get_svalue(msg, (gparam_t *)ip, &ip_address) < 0) {
+ LM_ERR("cannot get the IP address\n");
+ return -1;
Depends if you think there's any point of separating the various errors from
each other in the script of if it doesn't matter - the coder just want to know
that it failed, not caring about how. There are several functions in other
modules, like TM, that has multiple return values. It's really up to you, just
a suggestion that you consider this.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3802#pullrequestreview-1975734865
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3802/review/1975734...@github.com>_______________________________________________
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org
