encoder: Better support for Unix File Descriptors#269
encoder: Better support for Unix File Descriptors#269jsouthworth merged 2 commits intogodbus:masterfrom
Conversation
|
Thanks for doing this, it is a nice cleanup. There is one minor nit that I hope we can cleanup before committing the change. Would it be possible to avoid changing the public API of the existing methods and introduce new ones for those who need access to the FDs? This will avoid needing a "v6? revision to the API. |
message.go
Outdated
| // The possibly returned error can be an error of the underlying reader, an | ||
| // InvalidMessageError or a FormatError. | ||
| func DecodeMessage(rd io.Reader) (msg *Message, err error) { | ||
| func DecodeMessage(rd io.Reader, fds []int) (msg *Message, err error) { |
There was a problem hiding this comment.
Can we instead introduce this as DecodeMessageExtractingFDs and have the existing DecodeMessage function call that ?
There was a problem hiding this comment.
Absolutely! i'll write a new patch right away. thanks for looking at this!
message.go
Outdated
| func (msg *Message) EncodeTo(out io.Writer, order binary.ByteOrder) (err error) { | ||
| if err = msg.IsValid(); err != nil { | ||
| return | ||
| func (msg *Message) EncodeTo(out io.Writer, order binary.ByteOrder) (fds []int, err error) { |
There was a problem hiding this comment.
Can we name this EncodeToWithFDs and have the existing EncodeTo method signature call that. This allows old code to work transparently while providing the needed functionality here. We can note that one should use EncodeToWithFDs in the method documentation in future code (as is done elsewhere in this project).
This fixes a bug where UnixFDs that are inside structs and variants aren't handled properly. It fixes the decoder too. Fixes godbus#223
Also, restore functionality to message.EncodeTo and DecodeMessage This maintains compatibility with API v5
4abf7a1 to
7eea574
Compare
|
Looks good to me. Thanks! |
This fixes a bug where UnixFDs that are inside
structs and variants aren't handled properly. It fixes the decoder too.
Fixes #223