Fixing all megacheck errors.

9 files changed, 96 insertions, 124 deletions

diff --git a/table/destination.go b/table/destination.go
index bf375cd4..e0d0bc90 100644
--- a/table/destination.go
+++ b/table/destination.go
@@ -255,18 +255,6 @@ func (dd *Destination) GetMultiBestPath(id string) []*Path {
 	return getMultiBestPath(id, dd.knownPathList)
 }
 
-func (dd *Destination) validatePath(path *Path) {
-	if path == nil || path.GetRouteFamily() != dd.routeFamily {
-
-		log.WithFields(log.Fields{
-			"Topic":      "Table",
-			"Key":        dd.GetNlri().String(),
-			"Path":       path,
-			"ExpectedRF": dd.routeFamily,
-		}).Error("path is nil or invalid route family")
-	}
-}
-
 // Calculates best-path among known paths for this destination.
 //
 // Modifies destination's state related to stored paths. Removes withdrawn
@@ -530,10 +518,7 @@ func (dst *Destination) sort() {
 
 		better.reason = reason
 
-		if better == path1 {
-			return true
-		}
-		return false
+		return better == path1
 	})
 }
 
diff --git a/table/message.go b/table/message.go
index a7195ac6..8b9a4440 100644
--- a/table/message.go
+++ b/table/message.go
@@ -201,9 +201,7 @@ func UpdatePathAttrs4ByteAs(msg *bgp.BGPUpdate) error {
 	}
 
 	newIntfParams := make([]bgp.AsPathParamInterface, 0, len(asAttr.Value))
-	for _, p := range newParams {
-		newIntfParams = append(newIntfParams, p)
-	}
+	newIntfParams = append(newIntfParams, newParams...)
 
 	msg.PathAttributes[asAttrPos] = bgp.NewPathAttributeAsPath(newIntfParams)
 	return nil
@@ -384,7 +382,7 @@ func (p *packerV4) add(path *Path) {
 	if cages, y := p.hashmap[key]; y {
 		added := false
 		for _, c := range cages {
-			if bytes.Compare(c.attrsBytes, attrsB.Bytes()) == 0 {
+			if bytes.Equal(c.attrsBytes, attrsB.Bytes()) {
 				c.paths = append(c.paths, path)
 				added = true
 				break
diff --git a/table/message_test.go b/table/message_test.go
index f7cb6f5e..11c8dd19 100644
--- a/table/message_test.go
+++ b/table/message_test.go
@@ -578,9 +578,7 @@ func TestMergeV4NLRIs(t *testing.T) {
 	for _, msg := range msgs {
 		u := msg.Body.(*bgp.BGPUpdate)
 		assert.Equal(t, len(u.PathAttributes), 3)
-		for _, n := range u.NLRI {
-			l = append(l, n)
-		}
+		l = append(l, u.NLRI...)
 	}
 
 	assert.Equal(t, len(l), nr)
@@ -651,9 +649,7 @@ func TestMergeV4Withdraw(t *testing.T) {
 	for _, msg := range msgs {
 		u := msg.Body.(*bgp.BGPUpdate)
 		assert.Equal(t, len(u.PathAttributes), 0)
-		for _, n := range u.WithdrawnRoutes {
-			l = append(l, n)
-		}
+		l = append(l, u.WithdrawnRoutes...)
 	}
 	assert.Equal(t, len(l), nr)
 	for i, addr := range addrs {
diff --git a/table/path.go b/table/path.go
index 246f6aff..fa26f2e3 100644
--- a/table/path.go
+++ b/table/path.go
@@ -407,7 +407,7 @@ func (path *Path) SetAsLooped(y bool) {
 
 func (path *Path) IsLLGRStale() bool {
 	for _, c := range path.GetCommunities() {
-		if c == bgp.COMMUNITY_LLGR_STALE {
+		if c == uint32(bgp.COMMUNITY_LLGR_STALE) {
 			return true
 		}
 	}
@@ -798,7 +798,6 @@ func (path *Path) RemovePrivateAS(localAS uint32, option config.RemovePrivateAsO
 		}
 		path.setPathAttr(bgp.NewPathAttributeAsPath(newASParams))
 	}
-	return
 }
 
 func (path *Path) removeConfedAs() {
@@ -925,9 +924,7 @@ func (path *Path) GetExtCommunities() []bgp.ExtendedCommunityInterface {
 	eCommunityList := make([]bgp.ExtendedCommunityInterface, 0)
 	if attr := path.getPathAttr(bgp.BGP_ATTR_TYPE_EXTENDED_COMMUNITIES); attr != nil {
 		eCommunities := attr.(*bgp.PathAttributeExtendedCommunities).Value
-		for _, eCommunity := range eCommunities {
-			eCommunityList = append(eCommunityList, eCommunity)
-		}
+		eCommunityList = append(eCommunityList, eCommunities...)
 	}
 	return eCommunityList
 }
@@ -951,9 +948,7 @@ func (path *Path) GetLargeCommunities() []*bgp.LargeCommunity {
 	if a := path.getPathAttr(bgp.BGP_ATTR_TYPE_LARGE_COMMUNITY); a != nil {
 		v := a.(*bgp.PathAttributeLargeCommunities).Values
 		ret := make([]*bgp.LargeCommunity, 0, len(v))
-		for _, c := range v {
-			ret = append(ret, c)
-		}
+		ret = append(ret, v...)
 		return ret
 	}
 	return nil
@@ -979,20 +974,19 @@ func (path *Path) GetMed() (uint32, error) {
 
 // SetMed replace, add or subtraction med with new ones.
 func (path *Path) SetMed(med int64, doReplace bool) error {
-
 	parseMed := func(orgMed uint32, med int64, doReplace bool) (*bgp.PathAttributeMultiExitDisc, error) {
-		newMed := &bgp.PathAttributeMultiExitDisc{}
 		if doReplace {
-			newMed = bgp.NewPathAttributeMultiExitDisc(uint32(med))
-		} else {
-			if int64(orgMed)+med < 0 {
-				return nil, fmt.Errorf("med value invalid. it's underflow threshold.")
-			} else if int64(orgMed)+med > int64(math.MaxUint32) {
-				return nil, fmt.Errorf("med value invalid. it's overflow threshold.")
-			}
-			newMed = bgp.NewPathAttributeMultiExitDisc(uint32(int64(orgMed) + med))
+			return bgp.NewPathAttributeMultiExitDisc(uint32(med)), nil
+		}
+
+		medVal := int64(orgMed) + med
+		if medVal < 0 {
+			return nil, fmt.Errorf("med value invalid. it's underflow threshold: %v", medVal)
+		} else if medVal > int64(math.MaxUint32) {
+			return nil, fmt.Errorf("med value invalid. it's overflow threshold: %v", medVal)
 		}
-		return newMed, nil
+
+		return bgp.NewPathAttributeMultiExitDisc(uint32(int64(orgMed) + med)), nil
 	}
 
 	m := uint32(0)
diff --git a/table/policy.go b/table/policy.go
index da564abd..325121c0 100644
--- a/table/policy.go
+++ b/table/policy.go
@@ -26,12 +26,11 @@ import (
 	"strings"
 	"sync"
 
-	log "github.com/sirupsen/logrus"
-
-	radix "github.com/armon/go-radix"
-
 	"github.com/osrg/gobgp/config"
 	"github.com/osrg/gobgp/packet/bgp"
+
+	radix "github.com/armon/go-radix"
+	log "github.com/sirupsen/logrus"
 )
 
 type PolicyOptions struct {
@@ -310,6 +309,8 @@ func (p *Prefix) PrefixString() string {
 	return p.Prefix.String()
 }
 
+var _regexpPrefixRange = regexp.MustCompile(`(\d+)\.\.(\d+)`)
+
 func NewPrefix(c config.Prefix) (*Prefix, error) {
 	_, prefix, err := net.ParseCIDR(c.IpPrefix)
 	if err != nil {
@@ -325,28 +326,30 @@ func NewPrefix(c config.Prefix) (*Prefix, error) {
 		AddressFamily: rf,
 	}
 	maskRange := c.MasklengthRange
+
 	if maskRange == "" {
 		l, _ := prefix.Mask.Size()
 		maskLength := uint8(l)
 		p.MasklengthRangeMax = maskLength
 		p.MasklengthRangeMin = maskLength
-	} else {
-		exp := regexp.MustCompile("(\\d+)\\.\\.(\\d+)")
-		elems := exp.FindStringSubmatch(maskRange)
-		if len(elems) != 3 {
-			log.WithFields(log.Fields{
-				"Topic":           "Policy",
-				"Type":            "Prefix",
-				"MaskRangeFormat": maskRange,
-			}).Warn("mask length range format is invalid.")
-			return nil, fmt.Errorf("mask length range format is invalid")
-		}
-		// we've already checked the range is sane by regexp
-		min, _ := strconv.ParseUint(elems[1], 10, 8)
-		max, _ := strconv.ParseUint(elems[2], 10, 8)
-		p.MasklengthRangeMin = uint8(min)
-		p.MasklengthRangeMax = uint8(max)
+		return p, nil
 	}
+
+	elems := _regexpPrefixRange.FindStringSubmatch(maskRange)
+	if len(elems) != 3 {
+		log.WithFields(log.Fields{
+			"Topic":           "Policy",
+			"Type":            "Prefix",
+			"MaskRangeFormat": maskRange,
+		}).Warn("mask length range format is invalid.")
+		return nil, fmt.Errorf("mask length range format is invalid")
+	}
+
+	// we've already checked the range is sane by regexp
+	min, _ := strconv.ParseUint(elems[1], 10, 8)
+	max, _ := strconv.ParseUint(elems[2], 10, 8)
+	p.MasklengthRangeMin = uint8(min)
+	p.MasklengthRangeMax = uint8(max)
 	return p, nil
 }
 
@@ -817,32 +820,35 @@ func (m *singleAsPathMatch) Match(aspath []uint32) bool {
 	return false
 }
 
+var (
+	_regexpLeftMostRe = regexp.MustCompile(`$\^([0-9]+)_^`)
+	_regexpOriginRe   = regexp.MustCompile(`^_([0-9]+)\$$`)
+	_regexpIncludeRe  = regexp.MustCompile("^_([0-9]+)_$")
+	_regexpOnlyRe     = regexp.MustCompile(`^\^([0-9]+)\$$`)
+)
+
 func NewSingleAsPathMatch(arg string) *singleAsPathMatch {
-	leftMostRe := regexp.MustCompile("$\\^([0-9]+)_^")
-	originRe := regexp.MustCompile("^_([0-9]+)\\$$")
-	includeRe := regexp.MustCompile("^_([0-9]+)_$")
-	onlyRe := regexp.MustCompile("^\\^([0-9]+)\\$$")
 	switch {
-	case leftMostRe.MatchString(arg):
-		asn, _ := strconv.ParseUint(leftMostRe.FindStringSubmatch(arg)[1], 10, 32)
+	case _regexpLeftMostRe.MatchString(arg):
+		asn, _ := strconv.ParseUint(_regexpLeftMostRe.FindStringSubmatch(arg)[1], 10, 32)
 		return &singleAsPathMatch{
 			asn:  uint32(asn),
 			mode: LEFT_MOST,
 		}
-	case originRe.MatchString(arg):
-		asn, _ := strconv.ParseUint(originRe.FindStringSubmatch(arg)[1], 10, 32)
+	case _regexpOriginRe.MatchString(arg):
+		asn, _ := strconv.ParseUint(_regexpOriginRe.FindStringSubmatch(arg)[1], 10, 32)
 		return &singleAsPathMatch{
 			asn:  uint32(asn),
 			mode: ORIGIN,
 		}
-	case includeRe.MatchString(arg):
-		asn, _ := strconv.ParseUint(includeRe.FindStringSubmatch(arg)[1], 10, 32)
+	case _regexpIncludeRe.MatchString(arg):
+		asn, _ := strconv.ParseUint(_regexpIncludeRe.FindStringSubmatch(arg)[1], 10, 32)
 		return &singleAsPathMatch{
 			asn:  uint32(asn),
 			mode: INCLUDE,
 		}
-	case onlyRe.MatchString(arg):
-		asn, _ := strconv.ParseUint(onlyRe.FindStringSubmatch(arg)[1], 10, 32)
+	case _regexpOnlyRe.MatchString(arg):
+		asn, _ := strconv.ParseUint(_regexpOnlyRe.FindStringSubmatch(arg)[1], 10, 32)
 		return &singleAsPathMatch{
 			asn:  uint32(asn),
 			mode: ONLY,
@@ -1085,13 +1091,15 @@ func (s *CommunitySet) MarshalJSON() ([]byte, error) {
 	return json.Marshal(s.ToConfig())
 }
 
+var _regexpCommunity = regexp.MustCompile(`(\d+):(\d+)`)
+
 func ParseCommunity(arg string) (uint32, error) {
 	i, err := strconv.ParseUint(arg, 10, 32)
 	if err == nil {
 		return uint32(i), nil
 	}
-	exp := regexp.MustCompile("(\\d+):(\\d+)")
-	elems := exp.FindStringSubmatch(arg)
+
+	elems := _regexpCommunity.FindStringSubmatch(arg)
 	if len(elems) == 3 {
 		fst, _ := strconv.ParseUint(elems[1], 10, 16)
 		snd, _ := strconv.ParseUint(elems[2], 10, 16)
@@ -1136,24 +1144,25 @@ func ParseExtCommunity(arg string) (bgp.ExtendedCommunityInterface, error) {
 	return bgp.ParseExtendedCommunity(subtype, value)
 }
 
+var _regexpCommunity2 = regexp.MustCompile(`(\d+.)*\d+:\d+`)
+
 func ParseCommunityRegexp(arg string) (*regexp.Regexp, error) {
 	i, err := strconv.ParseUint(arg, 10, 32)
 	if err == nil {
-		return regexp.MustCompile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)), nil
+		return regexp.Compile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff))
 	}
-	if regexp.MustCompile("(\\d+.)*\\d+:\\d+").MatchString(arg) {
-		return regexp.MustCompile(fmt.Sprintf("^%s$", arg)), nil
+
+	if _regexpCommunity2.MatchString(arg) {
+		return regexp.Compile(fmt.Sprintf("^%s$", arg))
 	}
+
 	for i, v := range bgp.WellKnownCommunityNameMap {
 		if strings.Replace(strings.ToLower(arg), "_", "-", -1) == v {
-			return regexp.MustCompile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff)), nil
+			return regexp.Compile(fmt.Sprintf("^%d:%d$", i>>16, i&0x0000ffff))
 		}
 	}
-	exp, err := regexp.Compile(arg)
-	if err != nil {
-		return nil, fmt.Errorf("invalid community format: %s", arg)
-	}
-	return exp, nil
+
+	return regexp.Compile(arg)
 }
 
 func ParseExtCommunityRegexp(arg string) (bgp.ExtendedCommunityAttrSubType, *regexp.Regexp, error) {
@@ -1304,14 +1313,17 @@ func (s *LargeCommunitySet) MarshalJSON() ([]byte, error) {
 	return json.Marshal(s.ToConfig())
 }
 
+var _regexpCommunityLarge = regexp.MustCompile(`\d+:\d+:\d+`)
+
 func ParseLargeCommunityRegexp(arg string) (*regexp.Regexp, error) {
-	if regexp.MustCompile("\\d+:\\d+:\\d+").MatchString(arg) {
-		return regexp.MustCompile(fmt.Sprintf("^%s$", arg)), nil
+	if _regexpCommunityLarge.MatchString(arg) {
+		return regexp.Compile(fmt.Sprintf("^%s$", arg))
 	}
 	exp, err := regexp.Compile(arg)
 	if err != nil {
-		return nil, fmt.Errorf("invalid large-community format: %s", arg)
+		return nil, fmt.Errorf("invalid large-community format: %v", err)
 	}
+
 	return exp, nil
 }
 
@@ -2072,7 +2084,7 @@ func RegexpRemoveCommunities(path *Path, exps []*regexp.Regexp) {
 				break
 			}
 		}
-		if match == false {
+		if !match {
 			newComms = append(newComms, comm)
 		}
 	}
@@ -2095,7 +2107,7 @@ func RegexpRemoveExtCommunities(path *Path, exps []*regexp.Regexp, subtypes []bg
 				break
 			}
 		}
-		if match == false {
+		if !match {
 			newComms = append(newComms, comm)
 		}
 	}
@@ -2114,7 +2126,7 @@ func RegexpRemoveLargeCommunities(path *Path, exps []*regexp.Regexp) {
 				break
 			}
 		}
-		if match == false {
+		if !match {
 			newComms = append(newComms, comm)
 		}
 	}
@@ -2156,10 +2168,12 @@ func (a *CommunityAction) MarshalJSON() ([]byte, error) {
 	return json.Marshal(a.ToConfig())
 }
 
+// TODO: this is not efficient use of regexp, probably slow
+var _regexpCommunityReplaceString = regexp.MustCompile(`[\^\$]`)
+
 func (a *CommunityAction) String() string {
 	list := a.ToConfig().SetCommunityMethod.CommunitiesList
-	exp := regexp.MustCompile("[\\^\\$]")
-	l := exp.ReplaceAllString(strings.Join(list, ", "), "")
+	l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "")
 	return fmt.Sprintf("%s[%s]", a.action, l)
 }
 
@@ -2253,8 +2267,7 @@ func (a *ExtCommunityAction) ToConfig() *config.SetExtCommunity {
 
 func (a *ExtCommunityAction) String() string {
 	list := a.ToConfig().SetExtCommunityMethod.CommunitiesList
-	exp := regexp.MustCompile("[\\^\\$]")
-	l := exp.ReplaceAllString(strings.Join(list, ", "), "")
+	l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "")
 	return fmt.Sprintf("%s[%s]", a.action, l)
 }
 
@@ -2342,8 +2355,7 @@ func (a *LargeCommunityAction) ToConfig() *config.SetLargeCommunity {
 
 func (a *LargeCommunityAction) String() string {
 	list := a.ToConfig().SetLargeCommunityMethod.CommunitiesList
-	exp := regexp.MustCompile("[\\^\\$]")
-	l := exp.ReplaceAllString(strings.Join(list, ", "), "")
+	l := _regexpCommunityReplaceString.ReplaceAllString(strings.Join(list, ", "), "")
 	return fmt.Sprintf("%s[%s]", a.action, l)
 }
 
@@ -2432,12 +2444,14 @@ func (a *MedAction) MarshalJSON() ([]byte, error) {
 	return json.Marshal(a.ToConfig())
 }
 
+var _regexpParseMedAction = regexp.MustCompile(`^(\+|\-)?(\d+)$`)
+
 func NewMedAction(c config.BgpSetMedType) (*MedAction, error) {
 	if string(c) == "" {
 		return nil, nil
 	}
-	exp := regexp.MustCompile("^(\\+|\\-)?(\\d+)$")
-	elems := exp.FindStringSubmatch(string(c))
+
+	elems := _regexpParseMedAction.FindStringSubmatch(string(c))
 	if len(elems) != 3 {
 		return nil, fmt.Errorf("invalid med action format")
 	}
diff --git a/table/policy_test.go b/table/policy_test.go
index 007936c1..858e1098 100644
--- a/table/policy_test.go
+++ b/table/policy_test.go
@@ -24,11 +24,12 @@ import (
 	"testing"
 	"time"
 
-	log "github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/assert"
-
 	"github.com/osrg/gobgp/config"
 	"github.com/osrg/gobgp/packet/bgp"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPrefixCalcurateNoRange(t *testing.T) {
@@ -3060,6 +3061,7 @@ func TestLargeCommunityMatchAction(t *testing.T) {
 
 func TestAfiSafiInMatchPath(t *testing.T) {
 	condition, err := NewAfiSafiInCondition([]config.AfiSafiType{config.AFI_SAFI_TYPE_L3VPN_IPV4_UNICAST, config.AFI_SAFI_TYPE_L3VPN_IPV6_UNICAST})
+	require.NoError(t, err)
 
 	rtExtCom, err := bgp.ParseExtendedCommunity(bgp.EC_SUBTYPE_ROUTE_TARGET, "100:100")
 	assert.NoError(t, err)
diff --git a/table/table_manager.go b/table/table_manager.go
index 0bee18c3..26581cea 100644
--- a/table/table_manager.go
+++ b/table/table_manager.go
@@ -57,9 +57,7 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
 			reach = a
 		case *bgp.PathAttributeMpUnreachNLRI:
 			l := make([]bgp.AddrPrefixInterface, 0, len(a.Value))
-			for _, nlri := range a.Value {
-				l = append(l, nlri)
-			}
+			l = append(l, a.Value...)
 			dels = append(dels, l...)
 		default:
 			attrs = append(attrs, attr)
diff --git a/table/table_manager_test.go b/table/table_manager_test.go
index ee94d7e5..11a7b066 100644
--- a/table/table_manager_test.go
+++ b/table/table_manager_test.go
@@ -18,12 +18,10 @@ package table
 import (
 	_ "fmt"
 	"net"
-	"os"
 	"testing"
 	"time"
 
 	"github.com/osrg/gobgp/packet/bgp"
-	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -42,16 +40,6 @@ func (manager *TableManager) ProcessUpdate(fromPeer *PeerInfo, message *bgp.BGPM
 	return pathList, nil
 }
 
-func getLogger(lv log.Level) *log.Logger {
-	var l *log.Logger = &log.Logger{
-		Out:       os.Stderr,
-		Formatter: new(log.JSONFormatter),
-		Hooks:     make(map[log.Level][]log.Hook),
-		Level:     lv,
-	}
-	return l
-}
-
 func peerR1() *PeerInfo {
 	peer := &PeerInfo{
 		AS:      65000,
diff --git a/table/vrf.go b/table/vrf.go
index 480f963f..f58bee2a 100644
--- a/table/vrf.go
+++ b/table/vrf.go
@@ -30,10 +30,7 @@ type Vrf struct {
 func (v *Vrf) Clone() *Vrf {
 	f := func(rt []bgp.ExtendedCommunityInterface) []bgp.ExtendedCommunityInterface {
 		l := make([]bgp.ExtendedCommunityInterface, 0, len(rt))
-		for _, v := range rt {
-			l = append(l, v)
-		}
-		return l
+		return append(l, rt...)
 	}
 	return &Vrf{
 		Name:     v.Name,