Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/xml: report write error #3773

Closed
nsf opened this Issue Jun 23, 2012 · 5 comments

Comments

Projects
None yet
3 participants
@nsf
Copy link

nsf commented Jun 23, 2012

Just wandering what's the philosophy on handling I/O errors here, because there is a lot
of bufio.Writer method calls inside without errors being checked. As an example
Encoder.Encode method:

func (enc *Encoder) Encode(v interface{}) error {
        err := enc.marshalValue(reflect.ValueOf(v), nil)
        enc.Flush()
        return err
}

For a small amount of data if you write to a file on a disk with no free space on it -
it will return nil, even though enc.Flush() returned an error. In fact, let me make a
test case:

------------------------------------------------------------------------------
package main

import "encoding/xml"
import "errors"

// very bad, always returns an error                                                    
                            
type BadWriter int
func (BadWriter) Write(data []byte) (int, error) {
        return 0, errors.New("oh, no, please stop it")
}

type Dummy struct {
        Field int
}

func main() {
        enc := xml.NewEncoder(BadWriter(0))
        err := enc.Encode(Dummy{1})
        if err != nil {
                panic(err)
        }
        println("seems fine")
}
------------------------------------------------------------------------------

Seems "fine", eh?
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jun 24, 2012

Comment 1:

This is a bug. Note that since we use a bufio.Writer, we don't have to
check every method call, but we do need to check the error returned by
Flush at the end.
Russ

Status changed to Accepted.

@nsf

This comment has been minimized.

Copy link
Author

nsf commented Jun 25, 2012

Comment 2:

I disagree. Each Write method of the bufio.Writer may potentially call Flush.
@nsf

This comment has been minimized.

Copy link
Author

nsf commented Jun 25, 2012

Comment 3:

Oh, sorry, bufio.Writer sets 'err' field and then every Write and Flush call return it
if it's not nil as an error. We can use that fact to postpone error checking. Great.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jun 25, 2012

Comment 4:

Flush remembers errors. One check is enough.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jun 25, 2012

Comment 5:

This issue was closed by revision 32a0cbb.

Status changed to Fixed.

@nsf nsf added fixed labels Jun 25, 2012

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.