code review 157168: Initial syslog module. It's my first go, so please bare...

src/pkg/syslog/syslog.go

 // Copyright 2009 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 // The syslog package provides a simple interface to
-// the System Log service. It can send messages to the

[rsc, 2009/11/29 22:57:57]

s/System Log/system log/

[nictuku, 2009/12/07 18:43:51]

Done.

-// syslog daemon using UNIX sockets, or UDP and TCP

[rsc, 2009/11/29 22:57:57]

s/UNIX sockets/Unix domain sockets/

[nictuku, 2009/12/07 18:43:51]

Done.

-// streams.

[rsc, 2009/11/29 22:57:57]

s/streams/connections/

[nictuku, 2009/12/07 18:43:51]

Done.

+// the system log service. It can send messages to the
+// syslog daemon using UNIX domain sockets, UDP, or
+// TCP connections.
 package syslog
 
 import (
-        "io";

[rsc, 2009/11/29 22:57:57]

Please sort

[nictuku, 2009/12/07 18:43:51]

Done.

         "fmt";
+        "log";
         "net";
         "os";
-        "log";
-        "strings";
-        "strconv";
 )
+
+type Priority int
 
 const (
         // From /usr/include/sys/syslog.h.

[rsc, 2009/11/29 22:57:57]

Add 
// These are the same on Linux, BSD, and OS X.

Also probably define

type Priority int

and then make this 

LOG_EMERG Priority = iota;

[nictuku, 2009/12/07 18:43:51]

Done.

-        LOG_EMERG       = iota;
+        // These are the same on Linux, BSD, and OS X.
+        LOG_EMERG       Priority        = iota;
         LOG_ALERT;
         LOG_CRIT;
         LOG_ERR;
         LOG_WARNING;
         LOG_NOTICE;
         LOG_INFO;
         LOG_DEBUG;
 )
 
-// Syslog contains a message to be sent to the server, as well as optional

[rsc, 2009/11/29 22:57:57]

I'm not sure it makes sense to expose all this mechanism.
I think it would suffice to do

// New establishes a new connection to the system log daemon.
// Each write to the returned writer sends a log message with
// the given priority and prefix.
func New(priority Priority, prefix string) (w io.WriteCloser, err os.Error)

// Dial establishes a connection to a log daemon by connecting
// to address raddr on the network net. 
// Each write to the returned writer sends a log message with
// the given priority and prefix.
func Dial(net, raddr string, priority Priority, prefix string) (w
io.WriteCloser, err os.Error)

and have both of these return a *log

type log struct {
    prefix []byte;
    conn net.Conn;
}

with these methods

func (l *log) Close() os.Error
func (l *log) Write(b []byte) (n int, err os.Error)

Write would just create a new buffer with room for
prefix and b, copy them both in, and write it to conn.

[nictuku, 2009/12/07 18:43:51]

Done. Note that using []byte for prefix doesn't actually save any conversion. I
have to convert the whole thing to string so I can concatenate a "\n".  I would
keep it as string, but I'd rather hear what you think again.

-// routing settings.
-type Syslog struct {
-        // Log priority.
-        priority        int;
-        // Identifies which program is sending the message.
-        ident   string;
-        // Type of network address. Common values are "unixgram", "udp" and "tcp".
-        network string;
-        // Local address name, for example an IP address. Leaving blank is a safe choice.
-        laddr   string;
-        // Remote address name. For UDP and TCP, "host:port".
-        // For UNIX sockets, the path of the socket file.
-        raddr   string;
+// A Writer is a connection to a syslog server.
+type Writer struct {
+        priority        Priority;
+        prefix          string;
+        conn            net.Conn;
 }
 
-func (s Syslog) formatMsg(message []byte) []byte {
-        p := strconv.Itoa(s.priority);

[rsc, 2009/11/29 22:57:57]

This can be just %d below.

In terms of the interface sketched above, this should be

prefix := strings.Bytes("<%d>%s: ", priority, prefix);

[nictuku, 2009/12/07 18:43:51]

strings.Bytes doesn't do formatting, but I did a similar thing.

-        m := string(message);

[rsc, 2009/11/29 22:57:57]

The conversion here is unnecessary.
You can pass message (perhaps rename it to m)
directly to sprintf and it will format as a string
when using %s.

[nictuku, 2009/12/07 18:43:51]

Done. I got rid of this function and merged it into Write.

-        msg := fmt.Sprintf("<%s>%s: %s\n", p, s.ident, m);
-        b := strings.Bytes(msg);
-        return b;
+// New establishes a new connection to the system log daemon.
+// Each write to the returned writer sends a log message with
+// the given priority and prefix.
+func New(priority Priority, prefix string) (w *Writer, err os.Error) {
+        return Dial("", "", priority, prefix)
 }
 
-// Write sends the message to the System Log.
-func (s Syslog) Write(message []byte) (n int, err os.Error) {
-        if s.priority > LOG_DEBUG || s.priority < LOG_EMERG {
-                return 0, InvalidArgument("invalid priority")
+// Dial establishes a connection to a log daemon by connecting
+// to address raddr on the network net.
+// Each write to the returned writer sends a log message with
+// the given priority and prefix.
+func Dial(network, raddr string, priority Priority, prefix string) (w *Writer, err os.Error) {
+        if prefix == "" {
+                prefix = os.Args[0]
         }
-        if s.network == "" {
-                err = s.unixSyslog(message)
-        } else if s.raddr == "" {
-                return 0, InvalidArgument("network specified without raddr")
-
+        var conn net.Conn;
+        if network == "" {
+                conn, err = unixSyslog()
         } else {
-                err = s.send(message)
+                conn, err = net.Dial(network, "", raddr)
         }
-        if err != nil {
-                return 0, err
-        }
-        return len(message), nil;
+        return &Writer{priority, prefix, conn}, err;
 }
 
-func (s Syslog) send(message []byte) (err os.Error) {
-        if s.ident == "" {
-                s.ident = os.Args[0]
+func unixSyslog() (conn net.Conn, err os.Error) {
+        logTypes := []string{"unixgram", "unix"};
+        logPaths := []string{"/dev/log", "/var/run/syslog"};
+        var raddr string;
+        for _, network := range logTypes {
+                for _, path := range logPaths {
+                        raddr = path;
+                        conn, err := net.Dial(network, "", raddr);
+                        if err != nil {
+                                continue
+                        } else {
+                                return conn, nil
+                        }
+                }
         }
-        conn, err := net.Dial(s.network, s.laddr, s.raddr);
-        // TODO(yvesj): Re-use the connection.

[rsc, 2009/11/29 22:57:57]

Yes please - the interface sketched above makes this easy.

[nictuku, 2009/12/07 18:43:51]

Done.

-        defer conn.Close();
-        if err == nil {
-                b := s.formatMsg(message);
-                _, err = conn.Write(b);
+        return nil, os.ErrorString("Unix syslog delivery error");
+}
+
+// Write sends a log message to the syslog daemon.
+func (w *Writer) Write(b []byte) (int, os.Error) {
+        if w.priority > LOG_DEBUG || w.priority < LOG_EMERG {
+                return 0, os.EINVAL
         }
+        return fmt.Fprintf(w.conn, "<%d>%s: %s\n", w.priority, w.prefix, b);
+}
+
+func (w *Writer) writeString(p Priority, s string) (int, os.Error) {
+        return fmt.Fprintf(w.conn, "<%d>%s: %s\n", p, w.prefix, s)
+}
+
+func (w *Writer) Close() os.Error       { return w.conn.Close() }
+
+// Emerg logs a message using the LOG_EMERG priority.
+func (w *Writer) Emerg(m string) (err os.Error) {
+        _, err = w.writeString(LOG_EMERG, m);
+        return err;
+}
+// Crit logs a message using the LOG_CRIT priority.
+func (w *Writer) Crit(m string) (err os.Error) {
+        _, err = w.writeString(LOG_CRIT, m);
+        return err;
+}
+// ERR logs a message using the LOG_ERR priority.
+func (w *Writer) Err(m string) (err os.Error) {
+        _, err = w.writeString(LOG_ERR, m);
         return err;
 }
 
-func (s Syslog) unixSyslog(message []byte) (err os.Error) {
-        logTypes := []string{"unixgram", "unix"};
-        logPaths := []string{"/dev/log", "/var/run/syslog"};
-        // If successful, learn the configuration used.
-        for _, network := range logTypes {
-                s.network = network;
-                for _, path := range logPaths {
-                        s.raddr = path;
-                        err = s.send(message);
-                        if err != nil {
-                                continue
-                        } else {
-                                return nil
-                        }
-                }
-        }
-        // Forget what was learned since it's broken.
-        s.network = "";
-        s.raddr = "";
-        return os.ErrorString("Unix syslog delivery error");
-}
-
-
-// Convenience functions that logs a message using the requested severity.
-func (s Syslog) Emerg(message string) (err os.Error) {
-        s.priority = LOG_EMERG;
-        _, err = io.WriteString(s, message);
-        return err;
-}
-func (s Syslog) Crit(message string) (err os.Error) {
-        s.priority = LOG_CRIT;
-        _, err = io.WriteString(s, message);
+// Warning logs a message using the LOG_WARNING priority.
+func (w *Writer) Warning(m string) (err os.Error) {
+        _, err = w.writeString(LOG_WARNING, m);
         return err;
 }
 
-func (s Syslog) Err(message string) (err os.Error) {
-        s.priority = LOG_ERR;
-        _, err = io.WriteString(s, message);
+// Notice logs a message using the LOG_NOTICE priority.
+func (w *Writer) Notice(m string) (err os.Error) {
+        _, err = w.writeString(LOG_NOTICE, m);
+        return err;
+}
+// Info logs a message using the LOG_INFO priority.
+func (w *Writer) Info(m string) (err os.Error) {
+        _, err = w.writeString(LOG_INFO, m);
+        return err;
+}
+// Debug logs a message using the LOG_DEBUG priority.
+func (w *Writer) Debug(m string) (err os.Error) {
+        _, err = w.writeString(LOG_DEBUG, m);
         return err;
 }
 
-// DEPRECATED
-// Error = Err
-
-func (s Syslog) Warning(message string) (err os.Error) {
-        s.priority = LOG_WARNING;
-        _, err = io.WriteString(s, message);
-        return err;
+// NewLogger provides an object that implements the full log.Logger interface,
+// but sends messages to Syslog instead; flag is passed as is to Logger;
+// priority will be used for all messages sent using this interface.
+// All messages are logged with priority p.
+func NewLogger(p Priority, flag int) *log.Logger {
+        s, err := New(p, "");
+        if err != nil {
+                return nil
+        }
+        return log.New(s, nil, "", flag);
 }
-
-// DEPRECATED
-// Warn = Warning
-
-func (s Syslog) Notice(message string) (err os.Error) {
-        s.priority = LOG_NOTICE;
-        _, err = io.WriteString(s, message);
-        return err;
-}
-
-func (s Syslog) Info(message string) (err os.Error) {
-        s.priority = LOG_INFO;
-        _, err = io.WriteString(s, message);
-        return err;
-}
-
-func (s Syslog) Debug(message string) (err os.Error) {
-        s.priority = LOG_DEBUG;
-        _, err = io.WriteString(s, message);
-        return err;
-}
-
-// New creates a new Syslog object. It sets a default priority of LOG_INFO for
-// messages. Use one of the "Level" method levels to send messages using specific
-// priorities. For example:
-//
-// s := syslog.New();
-// s.Err("My Error")
-// s.Debug("Debug Message")
-//
-func New(ident string) *Syslog  { return &Syslog{LOG_INFO, ident, "", "", ""} }
-
-// NewLn creates a new Syslog object.
-// See that object documentation for details on the parameters;

[rsc, 2009/11/29 22:57:57]

The object documentation is invisible because the
fields are hidden.  If you run "godoc syslog" you can
see what the docs look like for this object.

[nictuku, 2009/12/07 18:43:51]

Deprecated this function in favor of Dial.

-func NewLn(ident string, priority int, net, laddr, raddr string) *Syslog {
-        return &Syslog{priority, ident, net, laddr, raddr}
-}
-
-// NewLogger provides an object that implements the full log.Logger interface,
-// but sends messages to Syslog instead; priority will be used for all messages
-// sent using this interface; if you need more control, use the Syslog interface
-// instead - see 'New' and 'NewLn'.
-func NewLogger(priority int) *log.Logger {

[rsc, 2009/11/29 22:57:57]

Maybe this should take two arguments

func NewLogger(p Priority, flag int)

and pass flag on to log.New, so that it is easy
to build loggers that crash or that include
the file and line number in the messages.

[nictuku, 2009/12/07 18:43:51]

Done.

-        s := Syslog{priority, "", "", "", ""};
-        return log.New(s, nil, "", 0);
-}
-
-type InvalidArgument string
-
-func (e InvalidArgument) String() string        { return "Invalid argument: " + string(e) }