WingNews logo WingNews GitHub [2]

joeshaw's comments

joeshaw | 1 year ago | on: Don't defer Close() on writable files (2017)

This is a contrived example, but imagine a situation where I have a file I want to write on disk and then have a reference to it in a database. If I have a flow like:

    func UpdateUser(user *User) (err error) {
        f, err := os.Create("/some/file.txt")
        if err != nil {
            return err
        }
        defer CheckClose(f, &err)

        if _, err := f.Write(somedata); err != nil {
            return err
        }

        if err := db.UpdateUser(user, "/some/file.txt"); err != nil {
            return err
        }

        return
    }
This function might have updated the user in the database with a new file despite the fact that `CheckClose` (defined up-thread) does check to see if the `Close` failed and returned an error. The calling code won't have known this has happened.

The core problem is that the error checking is not done soon enough, either because Go programmers are conditioned to `defer f.Close()` from nearly all example code -- most of it demonstrating reads, not writes -- or because they are handling the error, but only in a deferred function, not earlier.

A more correct way to do this would be:

    func UpdateUser(user *User) error {
        f, err := os.Create("/some/file.txt")
        if err != nil {
            return err
        }
        defer f.Close()

        if _, err := f.Write(somedata); err != nil {
            return err
        }

        if err := f.Sync(); err != nil {
            return err
        }

        if err := f.Close(); err != nil {
            return err
        }

        if err := db.UpdateUser(user, "/some/file.txt"); err != nil {
            return err
        }
    }
`Sync()` flushes the data to disk, and `Close()` gives a "last-chance" opportunity to return an error. The `defer f.Close()` exists as a way to ensure resource cleanup if an error occurs before the explicit `f.Close()` toward the end of the function. As I mentioned in an update to the post, double `Close()` is fine.

joeshaw | 1 year ago | on: Don't defer Close() on writable files (2017)

OP here. Another commenter pointed out that `errors.Join` didn't exist when I wrote this, but I wouldn't have changed my guidance if it had.

The core issue here is that you want to deal with errors as soon as possible. The async nature of writes to files makes this more challenging. Part of the purpose of this post was to inform people that you don't necessarily always see write errors at write-time, sometimes you see them at close-time and should handle them there. (And sometimes you don't see them at all without fsync.)

Even if you do handle errors in your deferred close, you may be taking other actions that you'd rather not have until the file i/o is completed, and this could leave your program in an inconsistent state. Side effects in Go are common, so this is a practical concern and it is hard to spot and debug.

joeshaw | 1 year ago | on: Don't defer Close() on writable files (2017)

I caution against this approach, as you are not really dealing with the error when it occurs. If the work you do after the defer has other side effects, you may have just gotten your application into an inconsistent state and it's very hard to see in code why this might be.

`defer` is really not well-suited for error handling, its benefit is mainly in resource cleanup where failure is impossible or doesn't matter. (This makes it fine for `Close` on read-only file I/O operations, and not so great for writes.)

joeshaw | 8 years ago | on: Go += Package Versioning

I'm fine with vendoring going away as long as we can check these new modules into version control and have vgo use them. Adding a caching proxy adds too much infrastructure for most folks, and without it you can't have reproducible and available builds.

I like Go's distributed package imports (even if everyone just uses Github), but it means you're distributing single points of failure all over the place. Vendoring (whether source or modules) solves this problem.

joeshaw | 8 years ago | on: Understanding Go panic output

In net/http, if a handler panics you don't want it to bring down your entire application. For more info, see https://golang.org/pkg/net/http/#Handler

The encoding/json package is interesting because it uses both panic() and recover() internally in a way more reminiscent to how exceptions are used in other languages (that is, for control flow).

For example, https://github.com/golang/go/blob/2596a0c075aeddec571cd658f7... panics, and https://github.com/golang/go/blob/2596a0c075aeddec571cd658f7... recovers it.

Both uses are quite uncommon, though.

joeshaw | 8 years ago | on: Understanding Go panic output

> I think there are plans to make an official debugger it just hasn't been prioritized.

There was a nascent project inside the Go team a couple of years ago (called ergo I think?) but I believe it was shelved in favor of Delve, which was much further along.

It's probably safe at this point to consider Delve the de facto standard Go debugger.

joeshaw | 8 years ago | on: Understanding Go panic output

My explanation for why is the following sentence:

> If a function could fail, the function must return an error as its last return value. The caller should immediately check for errors from that function.

In practice, in idiomatic code it is quite rare for a function that returns a `(* Something, error)` to return `(nil, nil)`, which are the cases where a nil pointer dereference would most often happen. The other case is not _immediately_ checking errors. I would say both of these cases are not idiomatic Go. (Yes, there are exceptions.)

The other cases where I've seen nil dereferences pop up most often:

(1) the zero value for maps is nil, and a nil map is mostly not usable. `m[50] = "foo"` will panic with a nil dereference if you've not initialized `m`. This is one of the most annoying things about Go.

(2) not checking for presence when pulling out of a map. If you have `map[string]* Something` and do `x := m["nonexistent"]` and try to use `x`, it'll blow up. Always use the `x, ok := m["nonexistent"]` form instead, and deal with stuff when `ok == false`.

(3) nil receivers. `var x *Something`, then `x.Method()` -- the `Method()` method _can_ deal when `x == nil`, but most code does not.

My reasons for citing C and Java:

C doesn't have multiple return values, and kinds of C code deals with errors differently. Lots of stuff just returns `NULL` and all too often things don't check for it. (`malloc()` is a perfect example of this -- it rarely fails but a _lot_ of code doesn't check for errors. Now extrapolate that to everything :)

For Java, it's because the try-catch control structure complicates program flow. If you have a large block of code in a try-catch, it is very easy for a different line of code to throw an exception than you expected, and as a result an object you attempt to dereference later is null and you get an NPE. There are ways to deal with this, of course, but in my experience most people are pretty lazy about the non-happy path.

EDIT: formatting. how can a site about programming have such terrible support for typing in code

joeshaw | 10 years ago | on: Mono Relicensed as MIT

Mono was started by Miguel at Ximian, and most of the Mono code was owned by Ximian. Ximian was bought by Novell and with it, all the Ximian-owned Mono code. (I don't remember if a CLA was required before or after Novell bought Ximian.)

The Novell-owned IP was either sold or licensed to Xamarin later.

joeshaw | 10 years ago | on: Show HN: “Who is hiring?” Map

It may correct itself as you zoom in. It seems to group geographic areas together in somewhat centralized locations to avoid having too many bubbles at a particular zoom level.

If you zoom in on the east coast of the US, for instance, jobs are grouped between NYC and Boston and the location of the bubble doesn't make much sense until you zoom in further and see them separated.

joeshaw | 11 years ago | on: Graph of Foursquare's Popularity After Removing Check-Ins

The app doesn't matter anymore. Twitter is the app now (and whomever else they license their points-of-interest service to). Consumers of their service likely feed back check-in-equivalent data to foursquare, which allows them to do trending and bolsters their database further.

I lament the loss of the app too. Checking in and becoming the mayor was the fun of the old app, and Swarm just doesn't do it for me. At this point I use it mostly as a log of new places I've been to so I can visit them again in the future, particularly while traveling.

joeshaw | 11 years ago | on: Graph of Foursquare's Popularity After Removing Check-Ins

Yes! I can't believe that nobody else has made this point.

I am as disappointed in the pivot of the app as everyone else, but it's hardly the sign of impending death the post (and many of these comments) make it out to be. 4sq built up an incredible points-of-interest database, and that is their real product.

By providing that POI data to other providers -- most notably Twitter -- they get much of the same data that checkins in their own app gave them. And Twitter is massively more popular than the Foursquare/Swarm apps ever was or ever will be.

joeshaw | 11 years ago | on: Ask HN: I got let go this morning. What should I do next?

I'm not sure what the market is like in Cleveland, but in Columbus there is a great demand for web developers. I work out of a coworking place there with several freelance web developers and I'd be happy to put you in touch with them if you're interested.

joeshaw | 11 years ago | on: Building Web Apps with Go

Use `goimports` as a save hook in your editor. It analyzes your code and inserts/removes import lines as needed.

Other than the occasional need to fix up an ambiguous import that it guessed incorrectly, I haven't touched import lines in my code in a year.

joeshaw | 11 years ago | on: Bash 'shellshock' scan of the Internet

The DHCP attack vector is the one that is scariest to me. We know that dhclient on Linux is vulnerable, but the number of unpatched Linux machines on a public wifi network will likely be too small to be worthwhile to try to capture. Macs on the other hand are prevalent in places like coffee shops, and Apple is notoriously slow to patch security vulnerabilities in their operating systems. Has anyone done much analysis on OS X's vulnerability to this bug?
page 1
more