1.4 post review

git diff 16529a00c72583560506597a1391627bd75f9515 origin/master

@mvavrusa

NEWS README THANKS doc man samples src/common src/knot/server src/libknot/dname*

@dsalzman

src/knot/conf src/knot/main.c src/libknot/util src/tests src/libknot/edns* src/libknot/rdata* src/libknot/common.h

@lslovak

src/libknot/updates src/libknot/zone src/knot/ctl src/libknot/dnssec

@jkadlec

src/utils src/libknot/packet src/libknot/nameserver src/libknot/dnssec

:honeybee: @jvcelak

src/knot/zone src/zscanner src/libknot/tsig* src/libknot/rrset* src/libknot/dnssec

Incorrect zone syntax for dnssec-keydir doc/reference.texi #722 (closed)

@jkadlec src/libknot/updates/ddns.c:1828 - The RRSIGs of the SOA should not be freed as the RRSet may have been inserted into the zone. Check however, if the change would not result in leaks with signing.

• doc/configuration.texi
  • I think an "online signing" is for generating signatures per-query, this should be named "automatic signing" maybe?
• doc/reference.texi
  • The zone_id and zones reference is just wrong, I don't know how, but fix it.
• man/knot.conf.5.in
  • I don't know how many people will get from the half-open interval bound "(7200, INT_MAX>" that the 7200 is NOT a valid value.

conf/conf.c #302 - If condition should be extended by negative values

• ixfr_fslimit is unsigned type and is wrongly set to negative value, it should be 0. Good catch! I'll fix this. @mvavrusa

• conf/remote.c:235
  • Both unused and checked by assert? It should return NULL if the result is invalid. @jvcelak

• knot/server/dthreads.c @jvcelak
  • There is no unit test for the added destructor.
  • I'd like to rather see a function to set destructor rather than a parameter for the _create functions, but okay. (discussed)

- libknot/zone/node.h @jkadlec

- In documentation of knot_node_rr_should_be_signed() It should be explicitly stated that it's not some RFC-compliant check, but rather an implementation-specific one.

• libknot/dname.h:163 reverse-fixed typo :bowtie:

libknot/dnssec/zone-sign.c:169 - Shouldn't it be a signed type (+ a check)? What if the result is negative?

jkadlec: No, it shouldn't. It will never be negative, unless someone changes system time to the first two hours of the epoch and has a signature lifetime under two hours, which is impossible for our implementation.

jvcelak: 72b5502b

I also object to moving rr_should_be_signed() into node.h. (libknot/zone/node.h:442). It should be somewhere in libknot/dnssec.

jkadlec: Fine.

Closing as all found problems have been addressed.

Status changed to closed