Various IO-related bug fixes #46
Conversation
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
As reference for myself and others
Note that the C errno value may be non-zero, and thus the err result may be non-nil, even if the function call is successful. Unlike normal Go conventions, you should first check whether the call succeeded before checking the error result. For example:
So this seems correct here overall.
| defer func() { d.err = nil }() | ||
| return total, d.err |
There was a problem hiding this comment.
Maybe it is just me and I know this is pre existing. But it is not obvious to me that this does work and return the error and at the same time set it to nil for *Data. I had to write a small test to confirm https://go.dev/play/p/9uC2vjLa_NU
Maybe it would be easier on reader to turn this into
err := d.err
d.err =nil
return total, err
There was a problem hiding this comment.
I’d slightly prefer the non-defer version as well, but I’ll let the package maintainer decide.
The function is not documented to, and does not, set it. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The function is not documented to, and does not, set it. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... per the primary return value. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Short writes are a valid outcome of gpgme_data_write, but invalid for io.Writer. Loop to conform to the interface. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
Thanks @mtrmac - I don't think changing the defer is enough to warrant another round. So I'll merge as-is. |
3 participants
errnoin calls that don’t set iterrno, inspect it only when it is known to be validio.Writer.Partially addresses #45 .