From: Alexander Barton Date: Sat, 26 Jan 2013 15:52:41 +0000 (+0100) Subject: Streamline punctuation of log messages X-Git-Tag: rel-21-rc1~151 X-Git-Url: https://arthur.barton.de/cgi-bin/gitweb.cgi?p=ngircd-alex.git;a=commitdiff_plain;h=d38d153f;hp=fd260404caa7ce174ada663024d19cd789c152b5 Streamline punctuation of log messages Make sure that all log messages end with a correct punctuation mark. The rules for formatting log messages are: 1. Add punctuation marks to all messages passed to the actual logging functions like Log() and LogDebug(). 2. Don't add any punctuation marks to messages that are stored in variables for later use or are passed over the network. 3. IP addresses, DNS host names and IRC server names should be quoted. 4. Messages originating in the network should be quoted (at least if they are "untrusted" or variable). Most probably this patch doesn't fix all mistakes, but it should be a good starting point ... --- diff --git a/src/ngircd/channel.c b/src/ngircd/channel.c index 00aafe05..4eab2726 100644 --- a/src/ngircd/channel.c +++ b/src/ngircd/channel.c @@ -131,11 +131,11 @@ Channel_InitPredefined( void ) new_chan = Channel_Create(conf_chan->name); if (!new_chan) { - Log(LOG_ERR, "Can't create pre-defined channel \"%s\"", + Log(LOG_ERR, "Can't create pre-defined channel \"%s\"!", conf_chan->name); continue; } - Log(LOG_INFO, "Created pre-defined channel \"%s\"", + Log(LOG_INFO, "Created pre-defined channel \"%s\".", conf_chan->name); Channel_ModeAdd(new_chan, 'P'); diff --git a/src/ngircd/client.c b/src/ngircd/client.c index 2114f84d..d10775a8 100644 --- a/src/ngircd/client.c +++ b/src/ngircd/client.c @@ -239,9 +239,9 @@ Client_Destroy( CLIENT *Client, const char *LogMsg, const char *FwdMsg, bool Sen assert( Client != NULL ); - if( LogMsg ) txt = LogMsg; - else txt = FwdMsg; - if( ! txt ) txt = "Reason unknown."; + txt = LogMsg ? LogMsg : FwdMsg; + if (!txt) + txt = "Reason unknown"; /* netsplit message */ if( Client->type == CLIENT_SERVER ) { @@ -281,10 +281,15 @@ Client_Destroy( CLIENT *Client, const char *LogMsg, const char *FwdMsg, bool Sen Destroy_UserOrService(c, txt, FwdMsg, SendQuit); else if( c->type == CLIENT_SERVER ) { - if( c != This_Server ) - { - if( c->conn_id != NONE ) Log( LOG_NOTICE|LOG_snotice, "Server \"%s\" unregistered (connection %d): %s", c->id, c->conn_id, txt ); - else Log( LOG_NOTICE|LOG_snotice, "Server \"%s\" unregistered: %s", c->id, txt ); + if (c != This_Server) { + if (c->conn_id != NONE) + Log(LOG_NOTICE|LOG_snotice, + "Server \"%s\" unregistered (connection %d): %s.", + c->id, c->conn_id, txt); + else + Log(LOG_NOTICE|LOG_snotice, + "Server \"%s\" unregistered: %s.", + c->id, txt); } /* inform other servers */ @@ -296,13 +301,19 @@ Client_Destroy( CLIENT *Client, const char *LogMsg, const char *FwdMsg, bool Sen } else { - if( c->conn_id != NONE ) - { - if( c->id[0] ) Log( LOG_NOTICE, "Client \"%s\" unregistered (connection %d): %s", c->id, c->conn_id, txt ); - else Log( LOG_NOTICE, "Client unregistered (connection %d): %s", c->conn_id, txt ); + if (c->conn_id != NONE) { + if (c->id[0]) + Log(LOG_NOTICE, + "Client \"%s\" unregistered (connection %d): %s.", + c->id, c->conn_id, txt); + else + Log(LOG_NOTICE, + "Client unregistered (connection %d): %s.", + c->conn_id, txt); } else { - Log(LOG_WARNING, "Unregistered unknown client \"%s\": %s", - c->id[0] ? c->id : "(No Nick)", txt ); + Log(LOG_WARNING, + "Unregistered unknown client \"%s\": %s", + c->id[0] ? c->id : "(No Nick)", txt); } } @@ -1424,7 +1435,7 @@ Destroy_UserOrService(CLIENT *Client, const char *Txt, const char *FwdMsg, bool if(Client->conn_id != NONE) { /* Local (directly connected) client */ Log(LOG_NOTICE, - "%s \"%s\" unregistered (connection %d): %s", + "%s \"%s\" unregistered (connection %d): %s.", Client_TypeText(Client), Client_Mask(Client), Client->conn_id, Txt); Log_ServerNotice('c', "Client exiting: %s (%s@%s) [%s]", @@ -1442,7 +1453,7 @@ Destroy_UserOrService(CLIENT *Client, const char *Txt, const char *FwdMsg, bool } } else { /* Remote client */ - LogDebug("%s \"%s\" unregistered: %s", + LogDebug("%s \"%s\" unregistered: %s.", Client_TypeText(Client), Client_Mask(Client), Txt); if(SendQuit) { diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index 80b085a8..142cf23a 100644 --- a/src/ngircd/conn.c +++ b/src/ngircd/conn.c @@ -205,7 +205,7 @@ cb_connserver(int sock, UNUSED short what) My_Connections[idx].host, Conf_Server[server].port, idx, strerror(err)); - Conn_Close(idx, "Can't connect!", NULL, false); + Conn_Close(idx, "Can't connect", NULL, false); if (ng_ipaddr_af(&Conf_Server[server].dst_addr[0])) { /* more addresses to try... */ @@ -282,7 +282,7 @@ cb_connserver_login_ssl(int sock, short unused) return; case -1: Log(LOG_ERR, "SSL connection on socket %d failed!", sock); - Conn_Close(idx, "Can't connect!", NULL, false); + Conn_Close(idx, "Can't connect", NULL, false); return; } @@ -568,7 +568,7 @@ InitSinaddrListenAddr(ng_ipaddr_t *addr, const char *listen_addrstr, UINT16 Port ret = ng_ipaddr_init(addr, listen_addrstr, Port); if (!ret) { assert(listen_addrstr); - Log(LOG_CRIT, "Can't bind to [%s]:%u: can't convert ip address \"%s\"", + Log(LOG_CRIT, "Can't bind to [%s]:%u: can't convert ip address \"%s\"!", listen_addrstr, Port, listen_addrstr); } return ret; @@ -631,7 +631,7 @@ NewListener(const char *listen_addr, UINT16 Port) return -1; if (bind(sock, (struct sockaddr *)&addr, ng_ipaddr_salen(&addr)) != 0) { - Log(LOG_CRIT, "Can't bind socket to address %s:%d - %s", + Log(LOG_CRIT, "Can't bind socket to address %s:%d - %s!", ng_ipaddr_tostr(&addr), Port, strerror(errno)); close(sock); return -1; @@ -1078,7 +1078,7 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const char *FwdMsg, bool InformClie Conn_OPTION_ADD( &My_Connections[Idx], CONN_ISCLOSING ); port = ng_ipaddr_getport(&My_Connections[Idx].addr); - Log(LOG_INFO, "Shutting down connection %d (%s) with %s:%d ...", Idx, + Log(LOG_INFO, "Shutting down connection %d (%s) with \"%s:%d\" ...", Idx, LogMsg ? LogMsg : FwdMsg, My_Connections[Idx].host, port); /* Search client, if any */ @@ -1152,7 +1152,7 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const char *FwdMsg, bool InformClie in_p = (int)(( in_k * 100 ) / in_z_k ); out_p = (int)(( out_k * 100 ) / out_z_k ); Log(LOG_INFO, - "Connection %d with %s:%d closed (in: %.1fk/%.1fk/%d%%, out: %.1fk/%.1fk/%d%%).", + "Connection %d with \"%s:%d\" closed (in: %.1fk/%.1fk/%d%%, out: %.1fk/%.1fk/%d%%).", Idx, My_Connections[Idx].host, port, in_k, in_z_k, in_p, out_k, out_z_k, out_p); } @@ -1160,7 +1160,7 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const char *FwdMsg, bool InformClie #endif { Log(LOG_INFO, - "Connection %d with %s:%d closed (in: %.1fk, out: %.1fk).", + "Connection %d with \"%s:%d\" closed (in: %.1fk, out: %.1fk).", Idx, My_Connections[Idx].host, port, in_k, out_k); } @@ -1500,7 +1500,7 @@ New_Connection(int Sock, UNUSED bool IsSSL) Client_SetHostname(c, My_Connections[new_sock].host); - Log(LOG_INFO, "Accepted connection %d from %s:%d on socket %d.", + Log(LOG_INFO, "Accepted connection %d from \"%s:%d\" on socket %d.", new_sock, My_Connections[new_sock].host, ng_ipaddr_getport(&new_addr), Sock); Account_Connection(); @@ -1629,13 +1629,10 @@ Read_Request( CONN_ID Idx ) #endif len = read(My_Connections[Idx].sock, readbuf, sizeof(readbuf)); if (len == 0) { - Log(LOG_INFO, "%s:%u (%s) is closing the connection ...", - My_Connections[Idx].host, - (unsigned int) ng_ipaddr_getport(&My_Connections[Idx].addr), - ng_ipaddr_tostr(&My_Connections[Idx].addr)); - Conn_Close(Idx, - "Socket closed!", "Client closed connection", - false); + LogDebug("Client \"%s:%u\" is closing connection %d ...", + My_Connections[Idx].host, + ng_ipaddr_tostr(&My_Connections[Idx].addr), Idx); + Conn_Close(Idx, NULL, "Client closed connection", false); return; } @@ -1643,7 +1640,7 @@ Read_Request( CONN_ID Idx ) if( errno == EAGAIN ) return; Log(LOG_ERR, "Read error on connection %d (socket %d): %s!", Idx, My_Connections[Idx].sock, strerror(errno)); - Conn_Close(Idx, "Read error!", "Client closed connection", + Conn_Close(Idx, "Read error", "Client closed connection", false); return; } diff --git a/src/ngircd/io.c b/src/ngircd/io.c index cce6ef53..dab30439 100644 --- a/src/ngircd/io.c +++ b/src/ngircd/io.c @@ -631,7 +631,7 @@ io_library_init_kqueue(unsigned int eventsize) io_masterfd = kqueue(); Log(LOG_INFO, - "IO subsystem: kqueue (initial maxfd %u, masterfd %d)", + "IO subsystem: kqueue (initial maxfd %u, masterfd %d).", eventsize, io_masterfd); if (io_masterfd >= 0) library_initialized = true; diff --git a/src/ngircd/irc-login.c b/src/ngircd/irc-login.c index 80a6627f..e7d83eff 100644 --- a/src/ngircd/irc-login.c +++ b/src/ngircd/irc-login.c @@ -691,11 +691,11 @@ IRC_QUIT( CLIENT *Client, REQUEST *Req ) } if (target != Client) { - Client_Destroy(target, "Got QUIT command.", + Client_Destroy(target, "Got QUIT command", Req->argc == 1 ? quitmsg : NULL, true); return CONNECTED; } else { - Conn_Close(Client_Conn(Client), "Got QUIT command.", + Conn_Close(Client_Conn(Client), "Got QUIT command", Req->argc == 1 ? quitmsg : NULL, true); return DISCONNECTED; } @@ -708,7 +708,7 @@ IRC_QUIT( CLIENT *Client, REQUEST *Req ) } /* User, Service, or not yet registered */ - Conn_Close(Client_Conn(Client), "Got QUIT command.", + Conn_Close(Client_Conn(Client), "Got QUIT command", Req->argc == 1 ? quitmsg : NULL, true); return DISCONNECTED; @@ -907,8 +907,9 @@ IRC_PONG(CLIENT *Client, REQUEST *Req) if (Client_Type(Client) == CLIENT_SERVER && Conn_LastPing(conn) == 0) { Log(LOG_INFO, - "Synchronization with \"%s\" done (connection %d): %ld seconds [%ld users, %ld channels]", + "Synchronization with \"%s\" done (connection %d): %ld second%s [%ld users, %ld channels].", Client_ID(Client), conn, time(NULL) - Conn_GetSignon(conn), + time(NULL) - Conn_GetSignon(conn) == 1 ? "" : "s", Client_UserCount(), Channel_CountVisible(NULL)); Conn_UpdatePing(conn); } else @@ -938,7 +939,7 @@ Kill_Nick(char *Nick, char *Reason) r.argv[1] = Reason; r.argc = 2; - Log(LOG_ERR, "User(s) with nick \"%s\" will be disconnected: %s", + Log(LOG_ERR, "User(s) with nick \"%s\" will be disconnected: %s!", Nick, Reason); IRC_KILL(Client_ThisServer(), &r); diff --git a/src/ngircd/irc-server.c b/src/ngircd/irc-server.c index a587c52f..0a9e930d 100644 --- a/src/ngircd/irc-server.c +++ b/src/ngircd/irc-server.c @@ -352,7 +352,7 @@ IRC_SQUIT(CLIENT * Client, REQUEST * Req) if (Req->argv[1][0]) if (Client_NextHop(from) != Client || con > NONE) - snprintf(msg, sizeof(msg), "%s (SQUIT from %s)", + snprintf(msg, sizeof(msg), "\"%s\" (SQUIT from %s)", Req->argv[1], Client_ID(from)); else strlcpy(msg, Req->argv[1], sizeof(msg)); @@ -385,7 +385,7 @@ IRC_SQUIT(CLIENT * Client, REQUEST * Req) logmsg[0] = '\0'; if (!strchr(msg, '(')) snprintf(logmsg, sizeof(logmsg), - "%s (SQUIT from %s)", Req->argv[1], + "\"%s\" (SQUIT from %s)", Req->argv[1], Client_ID(from)); Client_Destroy(target, logmsg[0] ? logmsg : msg, msg, false); diff --git a/src/ngircd/irc.c b/src/ngircd/irc.c index e76abcb8..37df0687 100644 --- a/src/ngircd/irc.c +++ b/src/ngircd/irc.c @@ -154,7 +154,7 @@ IRC_KILL( CLIENT *Client, REQUEST *Req ) if (Client != Client_ThisServer()) Log(LOG_NOTICE|LOG_snotice, - "Got KILL command from \"%s\" for \"%s\": %s", + "Got KILL command from \"%s\" for \"%s\": \"%s\".", Client_Mask(prefix), Req->argv[0], Req->argv[1]); /* Build reason string: Prefix the "reason" if the originator is a diff --git a/src/ngircd/ngircd.c b/src/ngircd/ngircd.c index dfae3366..e28c370b 100644 --- a/src/ngircd/ngircd.c +++ b/src/ngircd/ngircd.c @@ -535,16 +535,18 @@ Pidfile_Create(pid_t pid) len = snprintf(pidbuf, sizeof pidbuf, "%ld\n", (long)pid); if (len < 0 || len >= (int)sizeof pidbuf) { - Log(LOG_ERR, "Error converting pid"); + Log(LOG_ERR, "Error converting process ID!"); close(pidfd); return; } if (write(pidfd, pidbuf, (size_t)len) != (ssize_t)len) - Log( LOG_ERR, "Can't write PID file (%s): %s", Conf_PidFile, strerror( errno )); + Log(LOG_ERR, "Can't write PID file (%s): %s!", Conf_PidFile, + strerror(errno)); - if( close(pidfd) != 0 ) - Log( LOG_ERR, "Error closing PID file (%s): %s", Conf_PidFile, strerror( errno )); + if (close(pidfd) != 0) + Log(LOG_ERR, "Error closing PID file (%s): %s!", Conf_PidFile, + strerror(errno)); } /* Pidfile_Create */ @@ -678,14 +680,14 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) /* Change root */ if (Conf_Chroot[0]) { if (chdir(Conf_Chroot) != 0) { - Log(LOG_ERR, "Can't chdir() in ChrootDir (%s): %s", + Log(LOG_ERR, "Can't chdir() in ChrootDir (%s): %s!", Conf_Chroot, strerror(errno)); goto out; } if (chroot(Conf_Chroot) != 0) { Log(LOG_ERR, - "Can't change root directory to \"%s\": %s", + "Can't change root directory to \"%s\": %s!", Conf_Chroot, strerror(errno)); goto out; } else { @@ -716,7 +718,7 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) if (setgid(Conf_GID) != 0) { real_errno = errno; grp = getgrgid(Conf_GID); - Log(LOG_ERR, "Can't change group ID to %s(%u): %s", + Log(LOG_ERR, "Can't change group ID to %s(%u): %s!", grp ? grp->gr_name : "?", Conf_GID, strerror(errno)); if (real_errno != EPERM) @@ -730,7 +732,7 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) if (setuid(Conf_UID) != 0) { real_errno = errno; pwd = getpwuid(Conf_UID); - Log(LOG_ERR, "Can't change user ID to %s(%u): %s", + Log(LOG_ERR, "Can't change user ID to %s(%u): %s!", pwd ? pwd->pw_name : "?", Conf_UID, strerror(errno)); if (real_errno != EPERM) @@ -764,7 +766,7 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) setpgrp(0, getpid()); #endif if (chdir("/") != 0) - Log(LOG_ERR, "Can't change directory to '/': %s", + Log(LOG_ERR, "Can't change directory to '/': %s!", strerror(errno)); /* Detach stdin, stdout and stderr */ @@ -808,7 +810,7 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) pwd->pw_dir); else Log(LOG_INFO, - "Notice: Can't change working directory to \"%s\": %s", + "Notice: Can't change working directory to \"%s\": %s!", pwd->pw_dir, strerror(errno)); } else Log(LOG_ERR, "Can't get user informaton for UID %d!?", Conf_UID);