From 7830cf91663f071315c3d0406bc3de0f8a58c2b6 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:20:40 -0700 Subject: [PATCH 1/5] Don't use a strings.Reader where a bytes.Reader will do. There are several places where a []byte is converted to a string and then fed into strings.NewReader(). --- auth/auth.go | 3 ++- registry.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 045176ed48..e6aa048136 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -1,6 +1,7 @@ package auth import ( + "bytes" "encoding/base64" "encoding/json" "errors" @@ -111,7 +112,7 @@ func Login(authConfig *AuthConfig) (string, error) { return "", errors.New(errMsg) } - b := strings.NewReader(string(jsonBody)) + b := bytes.NewReader(jsonBody) req1, err := http.Post(REGISTRY_SERVER+"/v1/users", "application/json; charset=utf-8", b) if err != nil { errMsg = fmt.Sprintf("Server Error: %s", err) diff --git a/registry.go b/registry.go index 3e62ad96ee..4c51265981 100644 --- a/registry.go +++ b/registry.go @@ -1,6 +1,7 @@ package docker import ( + "bytes" "encoding/json" "fmt" "github.com/dotcloud/docker/auth" @@ -32,7 +33,7 @@ func NewImgJson(src []byte) (*Image, error) { func NewMultipleImgJson(src []byte) ([]*Image, error) { ret := []*Image{} - dec := json.NewDecoder(strings.NewReader(string(src))) + dec := json.NewDecoder(bytes.NewReader(src)) for { m := &Image{} if err := dec.Decode(m); err == io.EOF { @@ -226,7 +227,7 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth fmt.Fprintf(stdout, "Pushing %s metadata\n", img.Id) // FIXME: try json with UTF8 - jsonData := strings.NewReader(string(jsonRaw)) + jsonData := bytes.NewReader(jsonRaw) req, err := http.NewRequest("PUT", REGISTRY_ENDPOINT+"/images/"+img.Id+"/json", jsonData) if err != nil { return err From 15b30881570369564bde3f7d8faf4491b2ebfaab Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:22:56 -0700 Subject: [PATCH 2/5] Don't convert []byte to string unnecessarily. --- auth/auth.go | 4 ++-- container_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index e6aa048136..886d210f61 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -152,11 +152,11 @@ func Login(authConfig *AuthConfig) (string, error) { return "", errors.New(status) } } else { - status = fmt.Sprintf("Registration: %s", string(reqBody)) + status = fmt.Sprintf("Registration: %s", reqBody) return "", errors.New(status) } } else { - status = fmt.Sprintf("[%s] : %s", reqStatusCode, string(reqBody)) + status = fmt.Sprintf("[%s] : %s", reqStatusCode, reqBody) return "", errors.New(status) } if storeConfig { diff --git a/container_test.go b/container_test.go index fdc7f5703a..45d334ca22 100644 --- a/container_test.go +++ b/container_test.go @@ -227,7 +227,7 @@ func TestCommitRun(t *testing.T) { t.Fatal(err) } if string(output) != "hello\n" { - t.Fatalf("Unexpected output. Expected %s, received: %s (err: %s)", "hello\n", string(output), string(output2)) + t.Fatalf("Unexpected output. Expected %s, received: %s (err: %s)", "hello\n", output, output2) } } @@ -885,7 +885,7 @@ func BenchmarkRunSequencial(b *testing.B) { b.Fatal(err) } if string(output) != "foo" { - b.Fatalf("Unexecpted output: %v", string(output)) + b.Fatalf("Unexpected output: %s", output) } if err := runtime.Destroy(container); err != nil { b.Fatal(err) From c298a91f95dafb056cede9036f5e81f259b8f3e9 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:23:12 -0700 Subject: [PATCH 3/5] Use a *println or *print function instead of *printf where appropriate. --- commands.go | 14 +++++++------- container.go | 4 ++-- rcli/http.go | 2 +- rcli/tcp.go | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/commands.go b/commands.go index 105f76be5e..ad9688d9c8 100644 --- a/commands.go +++ b/commands.go @@ -146,12 +146,12 @@ func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...strin newAuthConfig := auth.NewAuthConfig(username, password, email, srv.runtime.root) status, err := auth.Login(newAuthConfig) if err != nil { - fmt.Fprintf(stdout, "Error : %s\n", err) + fmt.Fprintln(stdout, "Error:", err) } else { srv.runtime.authConfig = newAuthConfig } if status != "" { - fmt.Fprintf(stdout, status) + fmt.Fprint(stdout, status) } return nil } @@ -207,7 +207,7 @@ func (srv *Server) CmdInfo(stdin io.ReadCloser, stdout io.Writer, args ...string if !rcli.DEBUG_FLAG { return nil } - fmt.Fprintf(stdout, "debug mode enabled\n") + fmt.Fprintln(stdout, "debug mode enabled") fmt.Fprintf(stdout, "fds: %d\ngoroutines: %d\n", getTotalUsedFds(), runtime.NumGoroutine()) return nil } @@ -369,7 +369,7 @@ func (srv *Server) CmdHistory(stdin io.ReadCloser, stdout io.Writer, args ...str } w := tabwriter.NewWriter(stdout, 20, 1, 3, ' ', 0) defer w.Flush() - fmt.Fprintf(w, "ID\tCREATED\tCREATED BY\n") + fmt.Fprintln(w, "ID\tCREATED\tCREATED BY") return image.WalkHistory(func(img *Image) error { fmt.Fprintf(w, "%s\t%s\t%s\n", srv.runtime.repositories.ImageName(img.ShortId()), @@ -438,7 +438,7 @@ func (srv *Server) CmdImport(stdin io.ReadCloser, stdout io.Writer, args ...stri u.Host = src u.Path = "" } - fmt.Fprintf(stdout, "Downloading from %s\n", u.String()) + fmt.Fprintln(stdout, "Downloading from", u) // Download with curl (pretty progress bar) // If curl is not available, fallback to http.Get() resp, err = Download(u.String(), stdout) @@ -564,7 +564,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri } w := tabwriter.NewWriter(stdout, 20, 1, 3, ' ', 0) if !*quiet { - fmt.Fprintf(w, "REPOSITORY\tTAG\tID\tCREATED\tPARENT\n") + fmt.Fprintln(w, "REPOSITORY\tTAG\tID\tCREATED\tPARENT") } var allImages map[string]*Image var err error @@ -647,7 +647,7 @@ func (srv *Server) CmdPs(stdin io.ReadCloser, stdout io.Writer, args ...string) } w := tabwriter.NewWriter(stdout, 12, 1, 3, ' ', 0) if !*quiet { - fmt.Fprintf(w, "ID\tIMAGE\tCOMMAND\tCREATED\tSTATUS\tCOMMENT\n") + fmt.Fprintln(w, "ID\tIMAGE\tCOMMAND\tCREATED\tSTATUS\tCOMMENT") } for _, container := range srv.runtime.List() { if !container.State.Running && !*flAll { diff --git a/container.go b/container.go index a7094bf5ec..8f993a35e1 100644 --- a/container.go +++ b/container.go @@ -458,8 +458,8 @@ func (container *Container) Stop() error { // 1. Send a SIGTERM if output, err := exec.Command("lxc-kill", "-n", container.Id, "15").CombinedOutput(); err != nil { - log.Printf(string(output)) - log.Printf("Failed to send SIGTERM to the process, force killing") + log.Print(string(output)) + log.Print("Failed to send SIGTERM to the process, force killing") if err := container.Kill(); err != nil { return err } diff --git a/rcli/http.go b/rcli/http.go index cc8d3b149e..3eeb2c2a97 100644 --- a/rcli/http.go +++ b/rcli/http.go @@ -20,7 +20,7 @@ func ListenAndServeHTTP(addr string, service Service) error { func(w http.ResponseWriter, r *http.Request) { cmd, args := URLToCall(r.URL) if err := call(service, r.Body, &AutoFlush{w}, append([]string{cmd}, args...)...); err != nil { - fmt.Fprintf(w, "Error: "+err.Error()+"\n") + fmt.Fprintln(w, "Error:", err.Error()) } })) } diff --git a/rcli/tcp.go b/rcli/tcp.go index a1fa669023..ff7e191f42 100644 --- a/rcli/tcp.go +++ b/rcli/tcp.go @@ -51,8 +51,8 @@ func ListenAndServe(proto, addr string, service Service) error { CLIENT_SOCKET = conn } if err := Serve(conn, service); err != nil { - log.Printf("Error: " + err.Error() + "\n") - fmt.Fprintf(conn, "Error: "+err.Error()+"\n") + log.Println("Error:", err.Error()) + fmt.Fprintln(conn, "Error:", err.Error()) } conn.Close() }() From 13d2b086386196e9f4f03e4a18d508a65f6fc5ff Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:22:24 -0700 Subject: [PATCH 4/5] A few spelling/grammar corrections. --- auth/auth.go | 1 + commands.go | 2 +- graph.go | 20 ++++++++++---------- registry.go | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 886d210f61..2c282af218 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -131,6 +131,7 @@ func Login(authConfig *AuthConfig) (string, error) { status = "Account Created\n" storeConfig = true } else if reqStatusCode == 400 { + // FIXME: This should be 'exists', not 'exist'. Need to change on the server first. if string(reqBody) == "Username or email already exist" { client := &http.Client{} req, err := http.NewRequest("GET", REGISTRY_SERVER+"/v1/users", nil) diff --git a/commands.go b/commands.go index ad9688d9c8..a98a27cbb9 100644 --- a/commands.go +++ b/commands.go @@ -65,7 +65,7 @@ func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...strin // Read a line on raw terminal with support for simple backspace // sequences and echo. // - // This function is necessary because the login command must be done a + // This function is necessary because the login command must be done in a // raw terminal for two reasons: // - we have to read a password (without echoing it); // - the rcli "protocol" only supports cannonical and raw modes and you diff --git a/graph.go b/graph.go index ca126dc0e5..0f65aa205f 100644 --- a/graph.go +++ b/graph.go @@ -10,13 +10,13 @@ import ( "time" ) -// A Graph is a store for versioned filesystem images, and the relationship between them. +// A Graph is a store for versioned filesystem images and the relationship between them. type Graph struct { Root string idIndex *TruncIndex } -// NewGraph instanciates a new graph at the given root path in the filesystem. +// NewGraph instantiates a new graph at the given root path in the filesystem. // `root` will be created if it doesn't exist. func NewGraph(root string) (*Graph, error) { abspath, err := filepath.Abs(root) @@ -140,14 +140,14 @@ func (graph *Graph) Mktemp(id string) (string, error) { } // Garbage returns the "garbage", a staging area for deleted images. -// This allows images ot be deleted atomically by os.Rename(), instead of -// os.RemoveAll() which is prone to race conditions +// This allows images to be deleted atomically by os.Rename(), instead of +// os.RemoveAll() which is prone to race conditions. func (graph *Graph) Garbage() (*Graph, error) { return NewGraph(path.Join(graph.Root, ":garbage:")) } -// Check if given error is "not empty" -// Note: this is the way golang do it internally with os.IsNotExists +// Check if given error is "not empty". +// Note: this is the way golang does it internally with os.IsNotExists. func isNotEmpty(err error) bool { switch pe := err.(type) { case nil: @@ -190,7 +190,7 @@ func (graph *Graph) Delete(id string) error { return nil } -// Undelete moves an image back from the garbage to the main graph +// Undelete moves an image back from the garbage to the main graph. func (graph *Graph) Undelete(id string) error { garbage, err := graph.Garbage() if err != nil { @@ -203,7 +203,7 @@ func (graph *Graph) Undelete(id string) error { return nil } -// GarbageCollect definitely deletes all images moved to the garbage +// GarbageCollect definitely deletes all images moved to the garbage. func (graph *Graph) GarbageCollect() error { garbage, err := graph.Garbage() if err != nil { @@ -212,7 +212,7 @@ func (graph *Graph) GarbageCollect() error { return os.RemoveAll(garbage.Root) } -// Map returns a list of all images in the graph, addressable by ID +// Map returns a list of all images in the graph, addressable by ID. func (graph *Graph) Map() (map[string]*Image, error) { // FIXME: this should replace All() all, err := graph.All() @@ -226,7 +226,7 @@ func (graph *Graph) Map() (map[string]*Image, error) { return images, nil } -// All returns a list of all images in the graph +// All returns a list of all images in the graph. func (graph *Graph) All() ([]*Image, error) { var images []*Image err := graph.WalkAll(func(image *Image) { diff --git a/registry.go b/registry.go index 4c51265981..63cfd40350 100644 --- a/registry.go +++ b/registry.go @@ -21,7 +21,7 @@ func NewImgJson(src []byte) (*Image, error) { ret := &Image{} Debugf("Json string: {%s}\n", src) - // FIXME: Is there a cleaner way to "puryfy" the input json? + // FIXME: Is there a cleaner way to "purify" the input json? if err := json.Unmarshal(src, ret); err != nil { return nil, err } From 887f509d1dd2a5e34fbfb19ccd2c4b1eb3127dbd Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:36:27 -0700 Subject: [PATCH 5/5] Don't use an interface{} where a string will do. --- commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands.go b/commands.go index a98a27cbb9..0c896f29ba 100644 --- a/commands.go +++ b/commands.go @@ -28,7 +28,7 @@ func (srv *Server) Name() string { // FIXME: Stop violating DRY by repeating usage here and in Subcmd declarations func (srv *Server) Help() string { help := "Usage: docker COMMAND [arg...]\n\nA self-sufficient runtime for linux containers.\n\nCommands:\n" - for _, cmd := range [][]interface{}{ + for _, cmd := range [][]string{ {"attach", "Attach to a running container"}, {"commit", "Create a new image from a container's changes"}, {"diff", "Inspect changes on a container's filesystem"},