Merge pull request #79 from multiformats/nit/cleanup

small cleanups and optimizations

codec.go

@@ -11,7 +11,7 @@ func stringToBytes(s string) ([]byte, error) {
 	// consume trailing slashes
 	s = strings.TrimRight(s, "/")
 
-	b := new(bytes.Buffer)
+	var b bytes.Buffer
 	sp := strings.Split(s, "/")
 
 	if sp[0] != "" {
@@ -43,9 +43,6 @@ func stringToBytes(s string) ([]byte, error) {
 			sp = []string{"/" + strings.Join(sp, "/")}
 		}
 
-		if p.Transcoder == nil {
-			return nil, fmt.Errorf("no transcoder for %s protocol", p.Name)
-		}
 		a, err := p.Transcoder.StringToBytes(sp[0])
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse %s: %s %s", p.Name, sp[0], err)

interface.go

@@ -18,6 +18,8 @@ type Multiaddr interface {
 	Equal(Multiaddr) bool
 
 	// Bytes returns the []byte representation of this Multiaddr
+	//
+	// This function may expose immutable, internal state. Do not modify.
 	Bytes() []byte
 
 	// String returns the string representation of this Multiaddr

multiaddr.go

@@ -24,7 +24,7 @@ func NewMultiaddr(s string) (a Multiaddr, err error) {
 	if err != nil {
 		return nil, err
 	}
-	return &multiaddr{bytes: b}, nil
+	return multiaddr{bytes: b}, nil
 }
 
 // NewMultiaddrBytes initializes a Multiaddr from a byte representation.
@@ -41,34 +41,33 @@ func NewMultiaddrBytes(b []byte) (a Multiaddr, err error) {
 		return nil, err
 	}
 
-	return &multiaddr{bytes: b}, nil
+	return multiaddr{bytes: b}, nil
 }
 
 // Equal tests whether two multiaddrs are equal
-func (m *multiaddr) Equal(m2 Multiaddr) bool {
+func (m multiaddr) Equal(m2 Multiaddr) bool {
 	return bytes.Equal(m.bytes, m2.Bytes())
 }
 
 // Bytes returns the []byte representation of this Multiaddr
-func (m *multiaddr) Bytes() []byte {
+//
-	// consider returning copy to prevent changing underneath us?
+// Do not modify the returned buffer, it may be shared.
-	cpy := make([]byte, len(m.bytes))
+func (m multiaddr) Bytes() []byte {
-	copy(cpy, m.bytes)
+	return m.bytes
-	return cpy
 }
 
 // String returns the string representation of a Multiaddr
-func (m *multiaddr) String() string {
+func (m multiaddr) String() string {
 	s, err := bytesToString(m.bytes)
 	if err != nil {
-		panic("multiaddr failed to convert back to string. corrupted?")
+		panic(fmt.Errorf("multiaddr failed to convert back to string. corrupted? %s", err))
 	}
 	return s
 }
 
 // Protocols returns the list of protocols this Multiaddr has.
 // will panic in case we access bytes incorrectly.
-func (m *multiaddr) Protocols() []Protocol {
+func (m multiaddr) Protocols() []Protocol {
 	ps := make([]Protocol, 0, 8)
 	b := m.bytes
 	for len(b) > 0 {
@@ -97,18 +96,18 @@ func (m *multiaddr) Protocols() []Protocol {
 }
 
 // Encapsulate wraps a given Multiaddr, returning the resulting joined Multiaddr
-func (m *multiaddr) Encapsulate(o Multiaddr) Multiaddr {
+func (m multiaddr) Encapsulate(o Multiaddr) Multiaddr {
 	mb := m.bytes
 	ob := o.Bytes()
 
 	b := make([]byte, len(mb)+len(ob))
 	copy(b, mb)
 	copy(b[len(mb):], ob)
-	return &multiaddr{bytes: b}
+	return multiaddr{bytes: b}
 }
 
 // Decapsulate unwraps Multiaddr up until the given Multiaddr is found.
-func (m *multiaddr) Decapsulate(o Multiaddr) Multiaddr {
+func (m multiaddr) Decapsulate(o Multiaddr) Multiaddr {
 	s1 := m.String()
 	s2 := o.String()
 	i := strings.LastIndex(s1, s2)
@@ -116,7 +115,7 @@ func (m *multiaddr) Decapsulate(o Multiaddr) Multiaddr {
 		// if multiaddr not contained, returns a copy.
 		cpy := make([]byte, len(m.bytes))
 		copy(cpy, m.bytes)
-		return &multiaddr{bytes: cpy}
+		return multiaddr{bytes: cpy}
 	}
 
 	ma, err := NewMultiaddr(s1[:i])
@@ -128,7 +127,7 @@ func (m *multiaddr) Decapsulate(o Multiaddr) Multiaddr {
 
 var ErrProtocolNotFound = fmt.Errorf("protocol not found in multiaddr")
 
-func (m *multiaddr) ValueForProtocol(code int) (string, error) {
+func (m multiaddr) ValueForProtocol(code int) (string, error) {
 	for _, sub := range Split(m) {
 		p := sub.Protocols()[0]
 		if p.Code == code {

multiaddr_test.go

@@ -179,6 +179,7 @@ func TestStringToBytes(t *testing.T) {
 func TestBytesToString(t *testing.T) {
 
 	testString := func(s1 string, h string) {
+		t.Helper()
 		b, err := hex.DecodeString(h)
 		if err != nil {
 			t.Error("failed to decode hex", h)
@@ -229,12 +230,6 @@ func TestBytesSplitAndJoin(t *testing.T) {
 			t.Errorf("joined components failed: %s != %s", m, joined)
 		}
 
-		// modifying underlying bytes is fine.
-		m2 := m.(*multiaddr)
-		for i := range m2.bytes {
-			m2.bytes[i] = 0
-		}
-
 		for i, a := range split {
 			if a.String() != res[i] {
 				t.Errorf("split component failed: %s != %s", a, res[i])

util.go

@@ -11,13 +11,21 @@ func Split(m Multiaddr) []Multiaddr {
 
 	addrs := make([]Multiaddr, len(split))
 	for i, addr := range split {
-		addrs[i] = &multiaddr{bytes: addr}
+		addrs[i] = multiaddr{bytes: addr}
 	}
 	return addrs
 }
 
 // Join returns a combination of addresses.
 func Join(ms ...Multiaddr) Multiaddr {
+	switch len(ms) {
+	case 0:
+		// empty multiaddr, unfortunately, we have callers that rely on
+		// this contract.
+		return multiaddr{}
+	case 1:
+		return ms[0]
+	}
 
 	length := 0
 	bs := make([][]byte, len(ms))
@@ -29,21 +37,18 @@ func Join(ms ...Multiaddr) Multiaddr {
 	bidx := 0
 	b := make([]byte, length)
 	for _, mb := range bs {
-		for i := range mb {
+		bidx += copy(b[bidx:], mb)
-			b[bidx] = mb[i]
-			bidx++
-		}
 	}
-	return &multiaddr{bytes: b}
+	return multiaddr{bytes: b}
 }
 
 // Cast re-casts a byte slice as a multiaddr. will panic if it fails to parse.
 func Cast(b []byte) Multiaddr {
-	_, err := bytesToString(b)
+	m, err := NewMultiaddrBytes(b)
 	if err != nil {
 		panic(fmt.Errorf("multiaddr failed to parse: %s", err))
 	}
-	return &multiaddr{bytes: b}
+	return m
 }
 
 // StringCast like Cast, but parses a string. Will also panic if it fails to parse.