Zone events queue

Not good, if someone tried to access NULL data, it would not work - you need to fix in the rdata getter as well (knot_rdata_data - check for 0 size). EDIT: while you're at it, fix usage around rrset.c:464. EDIT 2: And to be really sure, knot_rdata_init() should be fixed too - do not copy into memory you got from knot_rdata_data() without check. There might be more places where we need to fix this.

• Q1: I don't think this is a problem, the function returns a pointer and the rdlen would be 0, so the caller should not read/write anything to that memory. This is the same behavior as with malloc()
• Q2: I'll fix that.
• Q3: zero-size memcpy is safe

okay

src/knot/nameserver/ixfr.c already reviewed I'll take:

• src/knot/server/journal.c:
  • Just changed some lens glare
  • retval check missing at line 238 (it should block, but anyway ...)
  • journal_create_file and journal_open_file are quite long
  • stuff moved from zones.c is a bit weird in general, removed the zones_ prefixes at least
  • since we plan to remove the journal though, I don't think any further changes are needed
    • I don't like the code, but it's already dead man walking issue 80. If you'd like to see any changes until 1.5, make it an issue, otherwise scratch that.
• src/knot/server/journal.h
  • Updated doxygen, please check.
• src/knot/nameserver/requestor.c
  • struct request not documented
  • 90: request_wait() could return -1, check missing the check should be only for 0 (no error, but no events)
  • requestor_dequeue, requestor_finished - could be static no, I have plans for it later
• src/knot/nameserver/requestor.h:
  • //knot_sign_context_t tsig; /* @note not implemented */ line 33 not sure what the current status is, but this does not seem right removed
  • structs and some params not documented / some extra params
• src/knot/nameserver/process_answer.c:
  • 119: A log message with TSIG failure would be nice
  • #undef ANSWER_DATA ? Not sure. :no_one_under_eighteen_symbol: this is used in the whole file, not local
• src/knot/nameserver/process_answer.h:
  • struct process_answer_param not documented
  • missing author I'm flattered.
• src/knot/zone/events.h:
  • Updated doxygen
• src/knot/zone/events.c:
  • Event implementations should be in an extra file
  • 97: magic chunk size 4096, also present at other places
  • 108: No SOA for AXFR? Could be included, if not bootstraping, right? does not make sense
  • 111: retval check missing, but it probably cannot fail it is optional
  • 221: Probably no need to init with time(0)
  • 250: Should't the function set payload to DNSSEC minimum instead of just warning? ok, that's probably better
  • 273: Doesn't this schedule NOTIFY for slaves as well? that's ok if slave servers have another tier of slave servers
  • 315: Maybe rotate masters no matter the result. no
  • 440: event_notify does not need to be an event at all, always scheduled to NOW needed to synchronize with other events
  • 604: Can this ever happen? it seems that yes
  • 787: Unused
  • 802: I don't see setting this variable back to false ... is that okay? for now, events are frozen before a zone is replaced

• src/knot/worker/pool.c
• src/knot/worker/pool.h
• src/knot/worker/queue.c
• src/knot/worker/queue.h
• src/knot/updates/apply.c
• src/knot/updates/apply.h
• src/knot/nameserver/internet.c

• src/knot/nameserver/update.h
• src/knot/nameserver/update.c
  • Is it OK to release 1.5 without forwarding?? (If it worked until now - did it?) it's issue 244, nonblocker for release candidate.
• src/knot/nameserver/axfr.h
  • Do we use symbol definition or #pragma once? In some other files there is #pragma, here there's #ifndef, etc.
    • #pragma once, old files weren't updated/issue 246
  • Incomplete doxygen comments (parameters, return values, more detailed descriptions). Some functions do not return normal KNOT return values, there it's even more necessary to explain the possible return values.
  • xfr_proc
    • Why not typedef it?
    • Is it necessary to have a list of trees in the structure? There may be at most 2 trees, why not put them directly? Reused for changesets in IXFR.
  • xfr_process_list()
    • Doesn't have axfr_ prefix. I know it's common for both AXFR and IXFR, but it hits in the eye. Together with the callback function type (xfr_put_cb). Shouldn't they be separated do some other file? Maybe later.
  • axfr_query() but axfr_process_answer() - maybe unify the names.
• src/knot/nameserver/axfr.c
  • #defines and #undefs do not match.
  • axfr_query_init()
    • RCU is locked, but it's not very clear if it's unlocked in every possible situation. (Not always the answer_cleanup() function is called.) Shouldn't it be locked on some higher level, possibly before calling axfr_query()? It is locked for each query, multi-packet processors lock it again for the whole operation.
  • axfr_query()
    • NS_NEED_ZONE() - is the RCODE right? What does this condition mean? That the server does not have the zone, or that it's not bootstraped yet? If the first one, it should be REFUSED, if the second, then I'm not sure what's the right RCODE. No, the NOTAUTH is right.
    • Shouldn't NS_NEED_AUTH() also set RCODE? It does.
    • Reserving space for TSIG: is there some check before calling this function, so that the space is not reserved, if TSIG is not required? There is a check afterwards, since the response is empty here and we don't know the key info yet.
  • axfr_answer_finalize()
    • Is freeing the contents necessary? Is it not done by the proc->ext_cleanup function after finishing the processing? :octocat: You don't want to free already switched contents.
• src/knot/updates/changesets.c
• src/knot/updates/changesets.h
• src/knot/updates/ddns.h
• src/knot/updates/ddns.c

Merged, separate issues #244 (closed) #245 (closed) #246 (closed) #247 (closed) (#80 (closed)) should be done post-merge.