code review 11737043: log/syslog: restore use of serverConn interface

log/syslog: restore use of serverConn interface

Revision 15629 (8d71734a0cb0) removed the serverConn interface
that was introduce in revision 7718 (ee5e80c62862).  The
serverConn interface was there for use by gccgo on Solaris,
and it is still needed there.  Solaris does not support
connecting to the syslog daemon over TCP, and gccgo simply
calls the C library function.  This CL restores the
interface.

[iant, 2013-07-23 21:18:41]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-07-23 21:22:28]

LGTM
 On Jul 23, 2013 2:18 PM, <iant@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> log/syslog: restore use of serverConn interface
>
> Revision 15629 (8d71734a0cb0) removed the serverConn interface
> that was introduce in revision 7718 (ee5e80c62862).  The
> serverConn interface was there for use by gccgo on Solaris,
> and it is still needed there.  Solaris does not support
> connecting to the syslog daemon over TCP, and gccgo simply
> calls the C library function.  This CL restores the
> interface.
>
> Please review this at
https://codereview.appspot.**com/11737043/<https://codereview.appspot.com/117...
>
> Affected files:
>   M src/pkg/log/syslog/syslog.go
>   M src/pkg/log/syslog/syslog_**unix.go
>
>
> Index: src/pkg/log/syslog/syslog.go
> ==============================**==============================**=======
> --- a/src/pkg/log/syslog/syslog.go
> +++ b/src/pkg/log/syslog/syslog.go
> @@ -88,6 +88,15 @@
>         raddr    string
>
>         mu   sync.Mutex // guards conn
> +       conn serverConn
> +}
> +
> +type serverConn interface {
> +       writeString(p Priority, hostname, tag, s, nl string) error
> +       close() error
> +}
> +
> +type netConn struct {
>         conn net.Conn
>  }
>
> @@ -135,7 +144,7 @@
>  func (w *Writer) connect() (err error) {
>         if w.conn != nil {
>                 // ignore err from close, it makes sense to continue anyway
> -               w.conn.Close()
> +               w.conn.close()
>                 w.conn = nil
>         }
>
> @@ -148,7 +157,7 @@
>                 var c net.Conn
>                 c, err = net.Dial(w.network, w.raddr)
>                 if err == nil {
> -                       w.conn = c
> +                       w.conn = netConn{c}
>                         if w.hostname == "" {
>                                 w.hostname = c.LocalAddr().String()
>                         }
> @@ -168,7 +177,7 @@
>         defer w.mu.Unlock()
>
>         if w.conn != nil {
> -               err := w.conn.Close()
> +               err := w.conn.close()
>                 w.conn = nil
>                 return err
>         }
> @@ -257,10 +266,7 @@
>                 nl = "\n"
>         }
>
> -       timestamp := time.Now().Format(time.**RFC3339)
> -       _, err := fmt.Fprintf(w.conn, "<%d>%s %s %s[%d]: %s%s",
> -               p, timestamp, w.hostname,
> -               w.tag, os.Getpid(), msg, nl)
> +       err := w.conn.writeString(p, w.hostname, w.tag, msg, nl)
>         if err != nil {
>                 return 0, err
>         }
> @@ -270,6 +276,18 @@
>         return len(msg), nil
>  }
>
> +func (n netConn) writeString(p Priority, hostname, tag, msg, nl string)
> error {
> +       timestamp := time.Now().Format(time.**RFC3339)
> +       _, err := fmt.Fprintf(n.conn, "<%d>%s %s %s[%d]: %s%s",
> +               p, timestamp, hostname,
> +               tag, os.Getpid(), msg, nl)
> +       return err
> +}
> +
> +func (n netConn) close() error {
> +       return n.conn.Close()
> +}
> +
>  // NewLogger creates a log.Logger whose output is written to
>  // the system log service with the specified priority. The logFlag
>  // argument is the flag set passed through to log.New to create
> Index: src/pkg/log/syslog/syslog_**unix.go
> ==============================**==============================**=======
> --- a/src/pkg/log/syslog/syslog_**unix.go
> +++ b/src/pkg/log/syslog/syslog_**unix.go
> @@ -14,7 +14,7 @@
>  // unixSyslog opens a connection to the syslog daemon running on the
>  // local machine using a Unix domain socket.
>
> -func unixSyslog() (conn net.Conn, err error) {
> +func unixSyslog() (conn serverConn, err error) {
>         logTypes := []string{"unixgram", "unix"}
>         logPaths := []string{"/dev/log", "/var/run/syslog"}
>         for _, network := range logTypes {
> @@ -23,7 +23,7 @@
>                         if err != nil {
>                                 continue
>                         } else {
> -                               return conn, nil
> +                               return netConn{conn}, nil
>                         }
>                 }
>         }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[rsc, 2013-07-23 21:24:21]

Please add a comment on serverConn explaining why it must be kept.

[iant, 2013-07-23 21:28:08]

On Tue, Jul 23, 2013 at 2:24 PM, Russ Cox <rsc@golang.org> wrote:
> Please add a comment on serverConn explaining why it must be kept.

Done.

PTAL.

Ian

[iant, 2013-07-23 21:29:12]

On Tue, Jul 23, 2013 at 2:24 PM, Russ Cox <rsc@golang.org> wrote:
> Please add a comment on serverConn explaining why it must be kept.

Done.

PTAL.

Ian

[iant, 2013-07-24 17:29:01]

*** Submitted as https://code.google.com/p/go/source/detail?r=afb591d143cd ***

log/syslog: restore use of serverConn interface

Revision 15629 (8d71734a0cb0) removed the serverConn interface
that was introduce in revision 7718 (ee5e80c62862).  The
serverConn interface was there for use by gccgo on Solaris,
and it is still needed there.  Solaris does not support
connecting to the syslog daemon over TCP, and gccgo simply
calls the C library function.  This CL restores the
interface.

R=golang-dev, bradfitz, rsc
CC=golang-dev
https://codereview.appspot.com/11737043