HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

Manipulating /etc/hosts file using Go

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filehostsmanipulatingusingetc

Problem

This is my first go at a Go program that's not a variation of "Hello {$name}". It's about twice as long as my previous Python implementation, and perhaps an order of magnitude messier.

The goal is to read in /etc/hosts and combine the various entries I've made over time:

127.0.0.1 host1.test
127.0.0.1 host2.test
111.111.111.111 staging.dev otherproject.dev
127.0.0.1 localhost host.test


into:

127.0.0.1 host.test host1.test host2.test localhost
111.111.111.111 otherproject.dev staging.dev


Any thoughts on how to make this cleaner, better, and/or more idiomatic?

```
package main

import (
"os"
"io/ioutil"
"strings"
"sort"
)

// Because this doesn't exist?
type StringSet struct {
set map[string]bool
}

func NewStringSet() StringSet {
return StringSet{ make(map[string]bool) }
}

// Returns true if added, false if already exists
func (s StringSet) Add(str string) bool {
_, found := s.set[str]
s.set[str] = true
return !found
}
// Returns the hostnames in alphabetical order
func (s StringSet) GetHostnames() []string {
keys := make([]string, 0, len(s.set))
for key := range s.set {
keys = append(keys, key)
}
sort.Strings(keys)
return keys
}

// Because this doesn't exist?
func pop(s []string) (string, []string) {
var x string
x, s = s[0], s[1:len(s)]
return x, s
}

// Returns the lines of a file
func GetLines(filename string) []string {
raw, err := ioutil.ReadFile("/etc/hosts")
if err != nil {
panic(err)
}
return strings.Split(string(raw[:]), "\n")
}

func WriteHostsFile(outfile string, hosts map[string]StringSet) bool {
// Range outputs a random order -.-
fd, err := os.Create(outfile)

if err != nil {
panic(err)
}

defer fd.Close()

fd.WriteString("# Managed by hosts.go\n")

for k := range hosts {
names := hosts[k].GetHostnames()
line := k + " " + strings.Join(names, " ")
fd.WriteString(line + "\n

Solution

First pass on your code:

  • Don't panic. For example, your WriteHostsFile function should return error instead of bool.



  • Don't create a special struct for a string set. Just use map[string]bool and exploit zero values =) Don't be afraid to use map[string]map[string]bool to represent your hosts.



  • Your Add function go away, but fyi: you are making it return a bool and never using it.



  • Your pop function could be simplified and inlined. Just use ip, parts = parts[0], parts[1:].



  • Check for nil value using foo == nil instead of len(foo) == 0.



  • strings.Split(line, " ") only works if things are separated with spaces — but indentations are a perfectly valid separator for /etc/hosts =)



  • If your lines aren't too long (and you can probably assume lines from /etc/hosts are going to be smaller than 65k chars), use bufio.Scanner to read a file line by line. See this SO answer for a full example and a discussion of caveats.



  • Your GetLines function should probably be inlined, but fyi: you're silently discarding its argument.



  • You probably want to use flags for the input and output paths, and output on stdout by default (maybe have a flag to override /etc/hosts). Outputting to a file named hosts in the current directory is pretty unexpected.



  • Instead of for k := range hosts and then calling hosts[k], you can use directly for k, h := range hosts and use h.



  • You don't want to write into a file line by line — what if you're overwriting the original file and your binary fails in the middle? You probably want to have WriteHostsFile write into a bytes.Buffer and write everything at once at the end.



  • All your functions are exported (they start with an uppercase letter), but this is not a library; so you shouldn't export anything: they should all start with a lowercase letter.



  • Your algorithm is not deterministic, because you're iterating over the elements of a map in WriteHostsFile: it might be unexpected to see the file modified if all hosts are already correctly grouped. Sorting the hosts by key; or by first appearance in the original file, would solve this.



Cheers!

Context

StackExchange Code Review Q#149498, answer score: 2

Revisions (0)

No revisions yet.