code review 961046: net: do not require newline at end of resolv.conf

net: do not require newline at end of resolv.conf

Fixes issue 686.

[rsc, 2010-04-28 22:23:38]

Hello adg (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[adg, 2010-04-29 01:06:55]

LGTM
the fix is fine, but it strikes me that net/parse.go is pretty general
and probably could live elsewhere (and be of use to other packages).

On 29 April 2010 08:23,  <rsc@golang.org> wrote:
> Reviewers: adg,
>
> Message:
> Hello adg (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> net: do not require newline at end of resolv.conf
>
> Fixes issue 686.
>
> Please review this at http://codereview.appspot.com/961046/show
>
> Affected files:
>  M src/pkg/net/parse.go
>
>
> Index: src/pkg/net/parse.go
> ===================================================================
> --- a/src/pkg/net/parse.go
> +++ b/src/pkg/net/parse.go
> @@ -35,6 +35,11 @@
>                        return
>                }
>        }
> +       if len(f.data) > 0 {
> +               s = string(data)
> +               f.data = nil
> +               ok = true
> +       }
>        return
>  }
>
>
>
>

[rsc, 2010-04-29 02:35:10]

On Wed, Apr 28, 2010 at 18:06, Andrew Gerrand <adg@golang.org> wrote:
> the fix is fine, but it strikes me that net/parse.go is pretty general
> and probably could live elsewhere (and be of use to other packages).

parse.go exists to avoid dependences on "elsewhere"
in such a low level package.

[rsc, 2010-04-29 02:36:08]

*** Submitted as http://code.google.com/p/go/source/detail?r=4e38b2d298dd ***

net: do not require newline at end of resolv.conf

Fixes issue 686.

R=adg
CC=golang-dev
http://codereview.appspot.com/961046

[cw, 2010-04-29 08:15:16]

NAK

http://codereview.appspot.com/961046/diff/8001/9001
File src/pkg/net/parse.go (right):

http://codereview.appspot.com/961046/diff/8001/9001#newcode39
src/pkg/net/parse.go:39: s = string(data)
is the end of the buffer is in the middle of the line this breaks things

LookupPort reading /etc/services for example is typically over 1K and many
things like tcp/80 aren't in the first 1K so all those tests now fail