GRONG: A small authoritative DNS name server written in Go

[Stéphane Bortzmeyer]

It is a not a serious production-ready name server (for instance, it
is very brittle when invalid packets come in) but I wrote it to learn
Go. Comments most welcome. Techniques used: goroutines, binary
protocol reading and writing.

(That's also why I don't use the dns* modules in package net.)

http://github.com/bortzmeyer/grong

[Russ Cox]

It's pretty nice looking code. A bunch of small comments below.
The most important one is please don't use runtime.Semacquire
when a channel will do just as well.

Russ

http://github.com/bortzmeyer/grong/blob/master/types.go#L28
Nice use of net.Addr.

http://github.com/bortzmeyer/grong/blob/master/types.go#L49
Qdcount etc should be implied by the length of the
corresponding slices.

Qsection and Asection might be clearer as Question and Answer,
with also NS and Extra added as the last two sections.

http://github.com/bortzmeyer/grong/blob/master/types.go#L63
There's no reason to misspell Type.

http://github.com/bortzmeyer/grong/blob/master/types.go#L112
ToTXT and Encode both allocate buffers.
Instead you could pass in the slice to write to and let the
marshaller have a single buffer instead of the small
allocations. That sidesteps the "what a waste" issue.
Another way would be to pass in a small bufio.Writer.

http://github.com/bortzmeyer/grong/blob/master/types.go#L132
Writing
for i := 0; i < len(s); i++ {
c := s[i]
instead of
for i, c := range strings.Bytes(s) {
will avoid an allocation.

You might be interested to look at packDomainName in
net/dnsmsg.go.

http://github.com/bortzmeyer/grong/blob/master/types.go#L155
It's probably cleaner to write to a bytes.Buffer than to
repeatedly call bytes.Add.

http://github.com/bortzmeyer/grong/blob/master/as112.go#L17
typical Go style is to uppercase the whole acronym
when camelcasing, so defaultTTL not defaultTtl

http://github.com/bortzmeyer/grong/blob/master/as112.go#L59
good place for a composite literal:

result[i] = types.RR{
Name: domain,
TTL: defaultTTL,
Type: types.NS,
Class: types.IN,
Data: types.Encode(text),
}

all the RR initializations could use them.

http://github.com/bortzmeyer/grong/blob/master/server.go#L48
The upper bounds are not relevant.
PutUint16(result[12+last+1:], ...)

http://github.com/bortzmeyer/grong/blob/master/server.go#L298
most programs shouldn't need to import "runtime".
there is no guarantee that Semacquire will stay as it is.
it's really a low-level thing, and in fact it might get
hidden again. on the other hand, channels are here to stay.

runtime.Semacquire instead of a channel may in fact
have less overhead but it almost never matters,
especially when the goal is to "sleep forever".

<-make(chan int)

http://github.com/bortzmeyer/grong/blob/master/as112.go#L90 "case
true" is more idiomatically written "default"

http://github.com/bortzmeyer/grong/blob/master/as112.go#L81
the switches would be more idiomatic if they
switched on the value on the left of all the ==.
(e.g., switch query.Qtype {, case types.NS, case types.SOA, default)

http://github.com/bortzmeyer/grong/blob/master/as112.go#L118
if all the cases that finished the result did "return result"
then you wouldn't need to set SERVFAIL in multiple places,
just once at the end of the function, unconditionally.