Use t.Errorf in tests and make the error variable naming more consistent. (#367)

* Make the error variable always named err.

Sometimes the error variable was named 'err' sometimes 'e'.  Sometimes
'e' refered to an EDNS or string and not an error type.

* Use t.Errorf instead of t.Logf & t.Fail.
This commit is contained in:
Michael Haro 2016-06-08 23:00:08 -07:00 committed by Miek Gieben
parent 48c8acaf0c
commit 1be7320498
10 changed files with 117 additions and 134 deletions

View File

@ -167,36 +167,31 @@ func TestClientEDNS0Local(t *testing.T) {
m.Extra = append(m.Extra, o)
c := new(Client)
r, _, e := c.Exchange(m, addrstr)
if e != nil {
t.Logf("failed to exchange: %s", e.Error())
t.Fail()
r, _, err := c.Exchange(m, addrstr)
if err != nil {
t.Errorf("failed to exchange: %s", err)
}
if r != nil && r.Rcode != RcodeSuccess {
t.Log("failed to get a valid answer")
t.Fail()
t.Error("failed to get a valid answer")
t.Logf("%v\n", r)
}
txt := r.Extra[0].(*TXT).Txt[0]
if txt != "Hello local edns" {
t.Log("Unexpected result for miek.nl", txt, "!= Hello local edns")
t.Fail()
t.Error("Unexpected result for miek.nl", txt, "!= Hello local edns")
}
// Validate the local options in the reply.
got := r.Extra[1].(*OPT).Option[0].(*EDNS0_LOCAL).String()
if got != optStr1 {
t.Logf("failed to get local edns0 answer; got %s, expected %s", got, optStr1)
t.Fail()
t.Errorf("failed to get local edns0 answer; got %s, expected %s", got, optStr1)
t.Logf("%v\n", r)
}
got = r.Extra[1].(*OPT).Option[1].(*EDNS0_LOCAL).String()
if got != optStr2 {
t.Logf("failed to get local edns0 answer; got %s, expected %s", got, optStr2)
t.Fail()
t.Errorf("failed to get local edns0 answer; got %s, expected %s", got, optStr2)
t.Logf("%v\n", r)
}
}
@ -305,12 +300,10 @@ func TestTruncatedMsg(t *testing.T) {
t.Errorf("unable to unpack message: %v", err)
}
if len(r.Answer) != cnt {
t.Logf("answer count after regular unpack doesn't match: %d", len(r.Answer))
t.Fail()
t.Errorf("answer count after regular unpack doesn't match: %d", len(r.Answer))
}
if len(r.Extra) != cnt {
t.Logf("extra count after regular unpack doesn't match: %d", len(r.Extra))
t.Fail()
t.Errorf("extra count after regular unpack doesn't match: %d", len(r.Extra))
}
m.Truncated = true
@ -324,16 +317,13 @@ func TestTruncatedMsg(t *testing.T) {
t.Errorf("unable to unpack truncated message: %v", err)
}
if !r.Truncated {
t.Log("truncated message wasn't unpacked as truncated")
t.Fail()
t.Errorf("truncated message wasn't unpacked as truncated")
}
if len(r.Answer) != cnt {
t.Logf("answer count after truncated unpack doesn't match: %d", len(r.Answer))
t.Fail()
t.Errorf("answer count after truncated unpack doesn't match: %d", len(r.Answer))
}
if len(r.Extra) != cnt {
t.Logf("extra count after truncated unpack doesn't match: %d", len(r.Extra))
t.Fail()
t.Errorf("extra count after truncated unpack doesn't match: %d", len(r.Extra))
}
// Now we want to remove almost all of the extra records
@ -357,16 +347,13 @@ func TestTruncatedMsg(t *testing.T) {
t.Errorf("unable to unpack cutoff message: %v", err)
}
if !r.Truncated {
t.Log("truncated cutoff message wasn't unpacked as truncated")
t.Fail()
t.Error("truncated cutoff message wasn't unpacked as truncated")
}
if len(r.Answer) != cnt {
t.Logf("answer count after cutoff unpack doesn't match: %d", len(r.Answer))
t.Fail()
t.Errorf("answer count after cutoff unpack doesn't match: %d", len(r.Answer))
}
if len(r.Extra) != 0 {
t.Logf("extra count after cutoff unpack is not zero: %d", len(r.Extra))
t.Fail()
t.Errorf("extra count after cutoff unpack is not zero: %d", len(r.Extra))
}
// Now we want to remove almost all of the answer records too
@ -391,12 +378,10 @@ func TestTruncatedMsg(t *testing.T) {
t.Errorf("unable to unpack cutoff message: %v", err)
}
if !r.Truncated {
t.Log("truncated cutoff message wasn't unpacked as truncated")
t.Fail()
t.Error("truncated cutoff message wasn't unpacked as truncated")
}
if len(r.Answer) != 0 {
t.Logf("answer count after second cutoff unpack is not zero: %d", len(r.Answer))
t.Fail()
t.Errorf("answer count after second cutoff unpack is not zero: %d", len(r.Answer))
}
// Now leave only 1 byte of the question
@ -406,8 +391,7 @@ func TestTruncatedMsg(t *testing.T) {
r = new(Msg)
err = r.Unpack(buf1)
if err == nil || err == ErrTruncated {
t.Logf("error should not be ErrTruncated from question cutoff unpack: %v", err)
t.Fail()
t.Errorf("error should not be ErrTruncated from question cutoff unpack: %v", err)
}
// Finally, if we only have the header, we should still return an error
@ -415,8 +399,7 @@ func TestTruncatedMsg(t *testing.T) {
r = new(Msg)
if err = r.Unpack(buf1); err == nil || err != ErrTruncated {
t.Logf("error not ErrTruncated from header-only unpack: %v", err)
t.Fail()
t.Errorf("error not ErrTruncated from header-only unpack: %v", err)
}
}

View File

@ -25,9 +25,9 @@ func (k *DNSKEY) NewPrivateKey(s string) (crypto.PrivateKey, error) {
// The public key must be known, because some cryptographic algorithms embed
// the public inside the privatekey.
func (k *DNSKEY) ReadPrivateKey(q io.Reader, file string) (crypto.PrivateKey, error) {
m, e := parseKey(q, file)
m, err := parseKey(q, file)
if m == nil {
return nil, e
return nil, err
}
if _, ok := m["private-key-format"]; !ok {
return nil, ErrPrivKey
@ -42,16 +42,16 @@ func (k *DNSKEY) ReadPrivateKey(q io.Reader, file string) (crypto.PrivateKey, er
}
switch uint8(algo) {
case DSA:
priv, e := readPrivateKeyDSA(m)
if e != nil {
return nil, e
priv, err := readPrivateKeyDSA(m)
if err != nil {
return nil, err
}
pub := k.publicKeyDSA()
if pub == nil {
return nil, ErrKey
}
priv.PublicKey = *pub
return priv, e
return priv, nil
case RSAMD5:
fallthrough
case RSASHA1:
@ -61,31 +61,31 @@ func (k *DNSKEY) ReadPrivateKey(q io.Reader, file string) (crypto.PrivateKey, er
case RSASHA256:
fallthrough
case RSASHA512:
priv, e := readPrivateKeyRSA(m)
if e != nil {
return nil, e
priv, err := readPrivateKeyRSA(m)
if err != nil {
return nil, err
}
pub := k.publicKeyRSA()
if pub == nil {
return nil, ErrKey
}
priv.PublicKey = *pub
return priv, e
return priv, nil
case ECCGOST:
return nil, ErrPrivKey
case ECDSAP256SHA256:
fallthrough
case ECDSAP384SHA384:
priv, e := readPrivateKeyECDSA(m)
if e != nil {
return nil, e
priv, err := readPrivateKeyECDSA(m)
if err != nil {
return nil, err
}
pub := k.publicKeyECDSA()
if pub == nil {
return nil, ErrKey
}
priv.PublicKey = *pub
return priv, e
return priv, nil
default:
return nil, ErrPrivKey
}

View File

@ -2,6 +2,7 @@ package dns
import (
"bytes"
"errors"
"fmt"
"strconv"
"strings"
@ -25,7 +26,7 @@ func generate(l lex, c chan lex, t chan *Token, o string) string {
if i+1 == len(l.token) {
return "bad step in $GENERATE range"
}
if s, e := strconv.Atoi(l.token[i+1:]); e == nil {
if s, err := strconv.Atoi(l.token[i+1:]); err == nil {
if s < 0 {
return "bad step in $GENERATE range"
}
@ -65,7 +66,7 @@ BuildRR:
escape bool
dom bytes.Buffer
mod string
err string
err error
offset int
)
@ -104,8 +105,8 @@ BuildRR:
return "bad modifier in $GENERATE"
}
mod, offset, err = modToPrintf(s[j+2 : j+2+sep])
if err != "" {
return err
if err != nil {
return err.Error()
}
j += 2 + sep // Jump to it
}
@ -119,9 +120,9 @@ BuildRR:
}
}
// Re-parse the RR and send it on the current channel t
rx, e := NewRR("$ORIGIN " + o + "\n" + dom.String())
if e != nil {
return e.(*ParseError).err
rx, err := NewRR("$ORIGIN " + o + "\n" + dom.String())
if err != nil {
return err.Error()
}
t <- &Token{RR: rx}
// Its more efficient to first built the rrlist and then parse it in
@ -131,28 +132,28 @@ BuildRR:
}
// Convert a $GENERATE modifier 0,0,d to something Printf can deal with.
func modToPrintf(s string) (string, int, string) {
func modToPrintf(s string) (string, int, error) {
xs := strings.SplitN(s, ",", 3)
if len(xs) != 3 {
return "", 0, "bad modifier in $GENERATE"
return "", 0, errors.New("bad modifier in $GENERATE")
}
// xs[0] is offset, xs[1] is width, xs[2] is base
if xs[2] != "o" && xs[2] != "d" && xs[2] != "x" && xs[2] != "X" {
return "", 0, "bad base in $GENERATE"
return "", 0, errors.New("bad base in $GENERATE")
}
offset, err := strconv.Atoi(xs[0])
if err != nil || offset > 255 {
return "", 0, "bad offset in $GENERATE"
return "", 0, errors.New("bad offset in $GENERATE")
}
width, err := strconv.Atoi(xs[1])
if err != nil || width > 255 {
return "", offset, "bad width in $GENERATE"
return "", offset, errors.New("bad width in $GENERATE")
}
switch {
case width < 0:
return "", offset, "bad width in $GENERATE"
return "", offset, errors.New("bad width in $GENERATE")
case width == 0:
return "%" + xs[1] + xs[2], offset, ""
return "%" + xs[1] + xs[2], offset, nil
}
return "%0" + xs[1] + xs[2], offset, ""
return "%0" + xs[1] + xs[2], offset, nil
}

26
msg.go
View File

@ -404,7 +404,6 @@ Loop:
}
func packTxt(txt []string, msg []byte, offset int, tmp []byte) (int, error) {
var err error
if len(txt) == 0 {
if offset >= len(msg) {
return offset, ErrBuf
@ -412,6 +411,7 @@ func packTxt(txt []string, msg []byte, offset int, tmp []byte) (int, error) {
msg[offset] = 0
return offset, nil
}
var err error
for i := range txt {
if len(txt[i]) > len(tmp) {
return offset, ErrBuf
@ -421,7 +421,7 @@ func packTxt(txt []string, msg []byte, offset int, tmp []byte) (int, error) {
return offset, err
}
}
return offset, err
return offset, nil
}
func packTxtString(s string, msg []byte, offset int, tmp []byte) (int, error) {
@ -598,8 +598,8 @@ func packStructValue(val reflect.Value, msg []byte, off int, compression map[str
case `dns:"opt"`: // edns
for j := 0; j < val.Field(i).Len(); j++ {
element := val.Field(i).Index(j).Interface()
b, e := element.(EDNS0).pack()
if e != nil || off+3 > lenmsg {
b, err := element.(EDNS0).pack()
if err != nil || off+3 > lenmsg {
return lenmsg, &Error{err: "overflow packing opt"}
}
// Option code
@ -762,9 +762,9 @@ func packStructValue(val reflect.Value, msg []byte, off int, compression map[str
default:
return lenmsg, &Error{"bad tag packing string: " + typefield.Tag.Get("dns")}
case `dns:"base64"`:
b64, e := fromBase64([]byte(s))
if e != nil {
return lenmsg, e
b64, err := fromBase64([]byte(s))
if err != nil {
return lenmsg, err
}
if off+len(b64) > lenmsg {
return lenmsg, &Error{err: "overflow packing base64"}
@ -793,9 +793,9 @@ func packStructValue(val reflect.Value, msg []byte, off int, compression map[str
msg[off-1] = 20
fallthrough
case `dns:"base32"`:
b32, e := fromBase32([]byte(s))
if e != nil {
return lenmsg, e
b32, err := fromBase32([]byte(s))
if err != nil {
return lenmsg, err
}
if off+len(b32) > lenmsg {
return lenmsg, &Error{err: "overflow packing base32"}
@ -806,9 +806,9 @@ func packStructValue(val reflect.Value, msg []byte, off int, compression map[str
fallthrough
case `dns:"hex"`:
// There is no length encoded here
h, e := hex.DecodeString(s)
if e != nil {
return lenmsg, e
h, err := hex.DecodeString(s)
if err != nil {
return lenmsg, err
}
if off+hex.DecodedLen(len(s)) > lenmsg {
return lenmsg, &Error{err: "overflow packing hex"}

View File

@ -306,9 +306,9 @@ func unpackStringBase64(msg []byte, off, end int) (string, int, error) {
}
func packStringBase64(s string, msg []byte, off int) (int, error) {
b64, e := fromBase64([]byte(s))
if e != nil {
return len(msg), e
b64, err := fromBase64([]byte(s))
if err != nil {
return len(msg), err
}
if off+len(b64) > len(msg) {
return len(msg), &Error{err: "overflow packing base64"}
@ -331,9 +331,9 @@ func unpackStringHex(msg []byte, off, end int) (string, int, error) {
}
func packStringHex(s string, msg []byte, off int) (int, error) {
h, e := hex.DecodeString(s)
if e != nil {
return len(msg), e
h, err := hex.DecodeString(s)
if err != nil {
return len(msg), err
}
if off+(len(h)) > len(msg) {
return len(msg), &Error{err: "overflow packing hex"}

View File

@ -70,16 +70,15 @@ func TestNormalizedString(t *testing.T) {
for tc, expected := range tests {
n := normalizedString(tc)
if n != expected {
t.Logf("expected %s, got %s", expected, n)
t.Fail()
t.Errorf("expected %s, got %s", expected, n)
}
}
}
func newRR(t *testing.T, s string) RR {
r, e := NewRR(s)
if e != nil {
t.Logf("newRR: %s", e)
r, err := NewRR(s)
if err != nil {
t.Logf("newRR: %v", err)
}
return r
}

View File

@ -377,8 +377,8 @@ func parseZone(r io.Reader, origin, f string, t chan *Token, include int) {
t <- &Token{Error: &ParseError{f, "expecting $GENERATE value, not this...", l}}
return
}
if e := generate(l, c, t, origin); e != "" {
t <- &Token{Error: &ParseError{f, e, l}}
if errMsg := generate(l, c, t, origin); errMsg != "" {
t <- &Token{Error: &ParseError{f, errMsg, l}}
return
}
st = zExpectOwnerDir
@ -966,8 +966,8 @@ func stringToNodeID(l lex) (uint64, *ParseError) {
return 0, &ParseError{l.token, "bad NID/L64 NodeID/Locator64", l}
}
s := l.token[0:4] + l.token[5:9] + l.token[10:14] + l.token[15:19]
u, e := strconv.ParseUint(s, 16, 64)
if e != nil {
u, err := strconv.ParseUint(s, 16, 64)
if err != nil {
return 0, &ParseError{l.token, "bad NID/L64 NodeID/Locator64", l}
}
return u, nil

View File

@ -321,20 +321,20 @@ func (srv *Server) ListenAndServe() error {
}
switch srv.Net {
case "tcp", "tcp4", "tcp6":
a, e := net.ResolveTCPAddr(srv.Net, addr)
if e != nil {
return e
a, err := net.ResolveTCPAddr(srv.Net, addr)
if err != nil {
return err
}
l, e := net.ListenTCP(srv.Net, a)
if e != nil {
return e
l, err := net.ListenTCP(srv.Net, a)
if err != nil {
return err
}
srv.Listener = l
srv.started = true
srv.lock.Unlock()
e = srv.serveTCP(l)
err = srv.serveTCP(l)
srv.lock.Lock() // to satisfy the defer at the top
return e
return err
case "tcp-tls", "tcp4-tls", "tcp6-tls":
network := "tcp"
if srv.Net == "tcp4-tls" {
@ -343,24 +343,24 @@ func (srv *Server) ListenAndServe() error {
network = "tcp6"
}
l, e := tls.Listen(network, addr, srv.TLSConfig)
if e != nil {
return e
l, err := tls.Listen(network, addr, srv.TLSConfig)
if err != nil {
return err
}
srv.Listener = l
srv.started = true
srv.lock.Unlock()
e = srv.serveTCP(l)
err = srv.serveTCP(l)
srv.lock.Lock() // to satisfy the defer at the top
return e
return err
case "udp", "udp4", "udp6":
a, e := net.ResolveUDPAddr(srv.Net, addr)
if e != nil {
return e
a, err := net.ResolveUDPAddr(srv.Net, addr)
if err != nil {
return err
}
l, e := net.ListenUDP(srv.Net, a)
if e != nil {
return e
l, err := net.ListenUDP(srv.Net, a)
if err != nil {
return err
}
if e := setUDPSocketOptions(l); e != nil {
return e
@ -368,9 +368,9 @@ func (srv *Server) ListenAndServe() error {
srv.PacketConn = l
srv.started = true
srv.lock.Unlock()
e = srv.serveUDP(l)
err = srv.serveUDP(l)
srv.lock.Lock() // to satisfy the defer at the top
return e
return err
}
return &Error{err: "bad network"}
}
@ -474,21 +474,21 @@ func (srv *Server) serveTCP(l net.Listener) error {
rtimeout := srv.getReadTimeout()
// deadline is not used here
for {
rw, e := l.Accept()
if e != nil {
if neterr, ok := e.(net.Error); ok && neterr.Temporary() {
rw, err := l.Accept()
if err != nil {
if neterr, ok := err.(net.Error); ok && neterr.Temporary() {
continue
}
return e
return err
}
m, e := reader.ReadTCP(rw, rtimeout)
m, err := reader.ReadTCP(rw, rtimeout)
srv.lock.RLock()
if !srv.started {
srv.lock.RUnlock()
return nil
}
srv.lock.RUnlock()
if e != nil {
if err != nil {
continue
}
srv.inFlight.Add(1)
@ -517,14 +517,14 @@ func (srv *Server) serveUDP(l *net.UDPConn) error {
rtimeout := srv.getReadTimeout()
// deadline is not used here
for {
m, s, e := reader.ReadUDP(l, rtimeout)
m, s, err := reader.ReadUDP(l, rtimeout)
srv.lock.RLock()
if !srv.started {
srv.lock.RUnlock()
return nil
}
srv.lock.RUnlock()
if e != nil {
if err != nil {
continue
}
srv.inFlight.Add(1)
@ -597,8 +597,8 @@ Exit:
if srv.IdleTimeout != nil {
idleTimeout = srv.IdleTimeout()
}
m, e := reader.ReadTCP(w.tcp, idleTimeout)
if e == nil {
m, err = reader.ReadTCP(w.tcp, idleTimeout)
if err == nil {
q++
goto Redo
}
@ -644,10 +644,10 @@ func (srv *Server) readTCP(conn net.Conn, timeout time.Duration) ([]byte, error)
func (srv *Server) readUDP(conn *net.UDPConn, timeout time.Duration) ([]byte, *SessionUDP, error) {
conn.SetReadDeadline(time.Now().Add(timeout))
m := make([]byte, srv.UDPSize)
n, s, e := ReadFromSessionUDP(conn, m)
if e != nil || n == 0 {
if e != nil {
return nil, nil, e
n, s, err := ReadFromSessionUDP(conn, m)
if err != nil || n == 0 {
if err != nil {
return nil, nil, err
}
return nil, nil, ErrShortRead
}

View File

@ -78,9 +78,9 @@ func TLSAName(name, service, network string) (string, error) {
if !IsFqdn(name) {
return "", ErrFqdn
}
p, e := net.LookupPort(network, service)
if e != nil {
return "", e
p, err := net.LookupPort(network, service)
if err != nil {
return "", err
}
return "_" + strconv.Itoa(p) + "._" + network + "." + name, nil
}

View File

@ -1260,9 +1260,9 @@ func TimeToString(t uint32) string {
// string values like "20110403154150" to an 32 bit integer.
// It takes serial arithmetic (RFC 1982) into account.
func StringToTime(s string) (uint32, error) {
t, e := time.Parse("20060102150405", s)
if e != nil {
return 0, e
t, err := time.Parse("20060102150405", s)
if err != nil {
return 0, err
}
mod := (t.Unix() / year68) - 1
if mod < 0 {