[ldns-users] ldns 1.7.0 rc3

[Michael Weiser]

Hi Willem,

On Thu, Dec 15, 2016 at 05:54:29PM +0100, Willem Toorop wrote:

> GOST support in newer OpenSSLs and an issue with how the language
> bindings deal with variables that have become const.

Since I was the one who created it, I today had a look at the
python-const situation. I get two sets of warnings:

1. ldns_tsig_credentials_struct:

ldns/tsig.h:28: Warning 451: Setting a const char * variable may leak memory.
ldns/tsig.h:29: Warning 451: Setting a const char * variable may leak memory.
ldns/tsig.h:30: Warning 451: Setting a const char * variable may leak memory.

These are from wrapping ldns_tsig_credentials_struct into a python
object. From what I understand it boils down to the fact that swig
doesn't dare free() the old value when setting a new one but also
doesn't dare not strdup()ing the new value.

This can be silenced  by something like this:

--- a/contrib/python/ldns.i
+++ b/contrib/python/ldns.i
@@ -126,6 +126,9 @@ uint32_t ldns_read_timeval_usec(struct timeval* t) {
 %immutable ldns_struct_rr_descriptor::_name;
 %immutable ldns_error_str;
 %immutable ldns_signing_algorithms;
+%immutable ldns_tsig_credentials_struct::algorithm;
+%immutable ldns_tsig_credentials_struct::keyname;
+%immutable ldns_tsig_credentials_struct::keydata;
 
 //*_new_frm_fp_l
 %apply int *OUTPUT { (int *line_nr) };

This turns ldns_tsig_credentials_struct read-only (no setter methods are
created) and in consequence most likely totally useless as a python
object. It could be used to safely interface to a bit of C code though
that initialised its own ldns_tsig_credentials_struct any odd way and
use the contents from python, if (I don't know how) this initialised
struct can be turned into a python object.

Alternatively a swig typemap could be written that either passes the
literal pointers around not caring about duplicating them or basically
ignores the const and restores behaviour of free()ing the values when
new ones are set. If the python binding is meant only ever to be used
*from* python, both solutions should be fine. But if someone wanted to
use it to interact with a bit of C code that might have done anything
to ldns_tsig_credentials_struct previously, it creates exactly the same
dilemma, swig is telling us about with this warning.

The clean way[tm] would then be to make the object opaque to C users as
well and have _init, _free and _set functions comparable to
ldns_resolver and other ldns objects.

Which begs the question I have been asking myself on the level of C
programming as well: What *is* ldns_tsig_credentials_struct actually
good for other than keeping those three parameters neatly together in
the user application? Are there actually any known users other than
examples/ldns-update.c? I'd expect most user applications to elect to
handle those parameters their own way anyway, leaving only potential for
confusion and boilerplate code...

So I have to ask: Might it be the simplest fix to simply drop the whole
concept? Suggested patch attached.

Or was the usage of ldns_tsig_credentials_struct supposed to be extended
to pass all three parameters to ldns in one go?

2. resolver:
./contrib/python/ldns_wrapper.c: In function '_ldns_resolver_tsig_algorithm':
./contrib/python/ldns_wrapper.c:3609:9: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     str = ldns_resolver_tsig_algorithm(res);
         ^
./contrib/python/ldns_wrapper.c: In function '_ldns_resolver_tsig_keydata':
./contrib/python/ldns_wrapper.c:3669:9: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     str = ldns_resolver_tsig_keydata(res);
         ^
./contrib/python/ldns_wrapper.c: In function '_ldns_resolver_tsig_keyname':
./contrib/python/ldns_wrapper.c:3680:9: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     str = ldns_resolver_tsig_keyname(res);

Those can easily be fixed like this, it seems:

diff --git a/contrib/python/ldns_resolver.i b/contrib/python/ldns_resolver.i
index b926e65a..7081ec36 100644
--- a/contrib/python/ldns_resolver.i
+++ b/contrib/python/ldns_resolver.i
@@ -113,9 +113,9 @@
 %rename(__ldns_resolver_tsig_algorithm) ldns_resolver_tsig_algorithm;
 %inline
 %{
-  char * _ldns_resolver_tsig_algorithm(const ldns_resolver *res)
+  const char * _ldns_resolver_tsig_algorithm(const ldns_resolver *res)
   {
-    char *str;
+    const char *str;
     str = ldns_resolver_tsig_algorithm(res);
     if (str != NULL) {
       str = strdup(str);

(full patch attached).

I hope this is helpful, if only perhaps as a second opinion. :)
-- 
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-const-warnings-for-tsig-parameters.patch
Type: text/x-diff
Size: 1817 bytes
Desc: not available
URL: <http://lists.nlnetlabs.nl/pipermail/ldns-users/attachments/20161218/7fb31c80/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Make-ldns_tsig_credentials_struct-read-only-for-pyth.patch
Type: text/x-diff
Size: 859 bytes
Desc: not available
URL: <http://lists.nlnetlabs.nl/pipermail/ldns-users/attachments/20161218/7fb31c80/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Drop-ldns_tsig_credentials.patch
Type: text/x-diff
Size: 8953 bytes
Desc: not available
URL: <http://lists.nlnetlabs.nl/pipermail/ldns-users/attachments/20161218/7fb31c80/attachment-0002.bin>