pkg/slayers: remove unnecessary sub-slicing

[jiceatscion]

Fixes: #4094

[jiceatscion]

This change is 

[matzf]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @oncilla)

---

pkg/slayers/path/scion/raw.go line 53 at r1 (raw file):

XXX(jiceatscion): Do we not want to update s.Raw anyway?

Not necessarily; in the (somewhat contrived) scenario that we read and parse a packet, modify the PathMeta header and then use SerializeTo to a new buffer to create a copy of the packet, this would inadvertently modify the original packet.

IMO, the root cause for this to even be a question is the redundancy of the path meta information which is contained both in PathMeta and Raw.
A solution to this would be to cleanly separate PathMeta and Raw; during DecodeFromBytes we could strip off the first MetaLen bytes from Raw, then the information is only in one place. During SerializeTo the only option will be

s.PathMeta.SerializeTo(b) // Serialize the PathMeta to b
copy(b[MetaLen:], s.Raw) // Serialize the rest of the path

Such a change would obviously affect a bunch of offset calculations in raw.go. Probably it would affect some test cases that create these objects by initializing the fields directly -- in a good way though, because it's currently possible to have very non-obviously conflicting information in the byte slices vs the PathMeta header.

[jiceatscion]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @oncilla)

---

pkg/slayers/path/scion/raw.go line 53 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

XXX(jiceatscion): Do we not want to update s.Raw anyway?

Not necessarily; in the (somewhat contrived) scenario that we read and parse a packet, modify the PathMeta header and then use SerializeTo to a new buffer to create a copy of the packet, this would inadvertently modify the original packet.

IMO, the root cause for this to even be a question is the redundancy of the path meta information which is contained both in PathMeta and Raw.
A solution to this would be to cleanly separate PathMeta and Raw; during DecodeFromBytes we could strip off the first MetaLen bytes from Raw, then the information is only in one place. During SerializeTo the only option will be

s.PathMeta.SerializeTo(b) // Serialize the PathMeta to b
copy(b[MetaLen:], s.Raw) // Serialize the rest of the path

Such a change would obviously affect a bunch of offset calculations in raw.go. Probably it would affect some test cases that create these objects by initializing the fields directly -- in a good way though, because it's currently possible to have very non-obviously conflicting information in the byte slices vs the PathMeta header.

Thanks! Since you approved, I take it that you agree with me that it can be treated as a separate issue. I will, add something to that effect to Dominik's comment instead of my question.

[jiceatscion]

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @matzf and @oncilla)

---

pkg/slayers/path/scion/raw.go line 53 at r1 (raw file):

Previously, jiceatscion wrote…

Thanks! Since you approved, I take it that you agree with me that it can be treated as a separate issue. I will, add something to that effect to Dominik's comment instead of my question.

I think my edited comment addresses your point.

[matzf]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @oncilla)