Skip to content

Commit

Permalink
Simplify and fix os.MkdirAll() usage
Browse files Browse the repository at this point in the history
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.

Quoting MkdirAll documentation:

> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.

This means two things:

1. If a directory to be created already exists, no error is
returned.

2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.

The above is a theory, based on quoted documentation and my UNIX
knowledge.
3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in moby#2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.

Because of moby#1, IsExist check after MkdirAll is not needed.

Because of moby#2 and moby#3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.

[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin authored and Christy Perez committed Aug 13, 2015
1 parent cb13dfc commit 8221beb
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 7 deletions.
7 changes: 1 addition & 6 deletions endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,7 @@ func (ep *endpoint) processOptions(options ...EndpointOption) {
}

func createBasePath(dir string) error {
err := os.MkdirAll(dir, 0644)
if err != nil && !os.IsExist(err) {
return err
}

return nil
return os.MkdirAll(dir, 0644)
}

func createFile(path string) error {
Expand Down
2 changes: 1 addition & 1 deletion sandbox/namespace_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func init() {

func createBasePath() {
err := os.MkdirAll(prefix, 0644)
if err != nil && !os.IsExist(err) {
if err != nil {
panic("Could not create net namespace path directory")
}

Expand Down

0 comments on commit 8221beb

Please sign in to comment.