c0defellas / enzo

core utilities
11 stars 2 forks source link

Add cat utility #1

Closed i4ki closed 7 years ago

i4ki commented 7 years ago

How we can make this code better? I mean decreasing the lines of code or making it more elegant.

I think this is a good way to we learn/improve reviewing each other's code. Some tools are really simple, requiring few minutes to write.

@patito @katcipis @lborguetti @rzanato

i4ki commented 7 years ago

ping

i4ki commented 7 years ago

Binary size:

Go version size size (undressed)
go1.4 1.9M 1.4M
go1.5 2.3M 1.6M
go1.6 2.3M 1.6M
go1.7 1.6M 1.0M
tip 1.6M 1004K

:)

i4ki commented 7 years ago

It turns out that "fmt" package is very big (in the context of this tiny software). Removing it, the new size table is:

Go version size size (undressed)
go1.4 1.1M 728K
go1.5 1.5M 1.0M
go1.6 1.5M 1.1M
go1.7 1.1M 688K
tip 1.1M 664K

For this reduction, the fatal function must be implemented as:

func fatal(err error) {
    os.Stderr.Write([]byte(err.Error()))
    os.Stderr.Write([]byte{0x0a})
    os.Exit(1)
}

What do guys you think? Worth it?

katcipis commented 7 years ago

If you remove fmt wont you loose all formatting capabilities ? Like fmt.Sprintf ? If that's the case...not SO sure if it is an awesome idea :-). We could re-implement this kind of formatting stuff as it is required too, sounds like fun.

i4ki commented 7 years ago

Here's the diff to remove the fmt:

https://github.com/tiago4orion/enzo/commit/1aef7db3c15a060eb527f36b182b3d2d5c5f7473

@katcipis What do you think of snippet below?

func errwrap(msg, fname string, err error) error {
        return errors.New(msg+" "+fname+": " + err.Error())
}

And then we can do:

if err != nil {
        return errwrap("error reading", name, err)
}

In this specific case (cat) I believe we can live without fmt. For other tools, I like the idea of implementing the stuff too if needed :)

The goal of the project is small static binaries. Then 1M -> 664K is a big win with little effort.

katcipis commented 7 years ago

Why:

    defer func() { f.Close() }()

And not:

    defer f.Close()
katcipis commented 7 years ago

Since the code is really small, it makes little sense to have fmt for few cases. Nothing against removing it.

katcipis commented 7 years ago

We have a lot of reading/writing on cat, what happens if stuff hangs ? It seems today that stays blocked forever, this is the intended behavior ?

i4ki commented 7 years ago

@katcipis about defer, it's dumb, sorry =) Probably I would use the func for more things and forgot...

I didn't understand your question regarding reading/writing. The correct behaviour (I think) must be:

$ cat # must block waiting for input (stdin) until CTRL-D is pressed
hello  <- stdin
hello -> stdout
world <- stdin
world -> stdout
^D
$ cat namedpipe # must block until something is written in the pipe (or it is closed)
^D
$

In case of any error, it must write the error to stdout and return an errored status code.

If it's not the case, is a misbehaviour.

katcipis commented 7 years ago

the question about hanging does not make sense on this case, sorry :-)