netlink: potential to optimize AttributeEncoder's buffer length calculation

[ubiquitousbyte]

This may seem rather overanalyzed, but I was reading through the code and found a potential pitfall in the String function of the AttributeEncoder type.
Isn't the following check necessary to prevent the length field of the attribute being appended from overflowing?

if len(s) > math.MaxUint16-nlaHeaderLen {
   ae.err = errors.New("Attribute length overflow")
   return 
}
 

[mdlayher]

It seems you're correct! A length check should be added to each of Bytes, Do, and String. Want to send a PR?

[ubiquitousbyte]

Sure thing, mate. I'll send the PR tomorrow after work.

I'm also seeing a small performance optimization that we can integrate without affecting code readability.
The AttributeEncoder can keep track of the total (aligned) buffer length and gradually increment it whenever the caller issues an append operation (via Bytes, String, Int8..64 etc).

This will effectively make this for-loop redundant, because we've already computed the length. Thus, marshalling a slice of attributes via the encoder is now an O(n) operation, instead of O(n^2).

Note that this will require refactoring the Encode function, as well as the relevant bits from MarshalAttributes into a separate function.

What do you think?

[mdlayher]

Internal improvements are always welcome, as long as they don't dramatically impact the readability of the code! I'd be happy to take a look at what you have in mind.

[mdlayher]

I've committed a fix for the length overflow issue, but will retitle this for the performance improvement issue.