openssh unix dev - Jan 2018 - sshfp/ldns still having issues in 7.6

> I replaced the ldns code with getdns. Works fine for more than a year now.
>
I am interested in how you did that. Would you mind sharing your procedure?


> I don't think anybody cares. I tried to tell people. But that had no
> effect.
>
There certainly is not as much talk about it as I would expect there to be.

In your letter dated Wed, 10 Jan 2018 17:11:00 -0700 you
wrote:
>> I replaced the ldns code with getdns. Works fine for more than a year
now.
>
>I am interested in how you did that. Would you mind sharing your procedure?
I rewrote the code that uses ldns to use getdns instead.

The code is on github:
https://github.com/phicoh/openssh-getdns/tree/github-getdns-7.5

(I guess I should update to 7.6 at some point).

On Thu, Jan 11, 2018 at 3:31 AM, Philip Homburg <pch-openssh at
u-1.phicoh.com>
wrote:

> I rewrote the code that uses ldns to use getdns instead.
>
> The code is on github:
> https://github.com/phicoh/openssh-getdns/tree/github-getdns-7.5
>
> Well this looks handy. Thanks for pointing me that way. I will take a look
at it.

On 11 January 2018 at 18:31, Philip Homburg <pch-openssh at
u-1.phicoh.com> wrote:
[...]
> I rewrote the code that uses ldns to use getdns instead.
>
> The code is on github:
> https://github.com/phicoh/openssh-getdns/tree/github-getdns-7.5
Looking at this commit:
https://github.com/phicoh/openssh-getdns/commit/7ea7ca559f30147192263b57d9ee61d9ddf4f38d
it looks ok. Some comments:

+ [...] unsigned int rdtype,  [...]
+ int answer_type;
[...]
+ this_ret= getdns_dict_get_int(this_answer, "type",
&answer_type);
[...]
+ if ((unsigned)answer_type != rdtype)
+         continue;

I suspect you put that cast there to suppress a signed-comparison warning.
getdns_dict_get_int seems to take uint32_t as its third arg and I
think the above will do the wrong thing on any system where ints are
not 32bits.

+ if (rrset->rri_rdatas[i].rdi_data == NULL)
+         break;
+ free(rrset->rri_rdatas[i].rdi_data);

and similarly

+ if (rrset->rri_name)
+         free(rrset->rri_name);

The C standard (and POSIX) guarantees that free(NULL) is a no-op, you
can simplify those by removing the conditionals.

You have some inconsistent style (braces, spacing).  OpenSSH follows
the OpenBSD style guide (http://man.openbsd.org/style).

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.