Changeset View
Standalone View
usr.sbin/pw/pw_log.c
Show All 23 Lines | |||||
* SUCH DAMAGE. | * SUCH DAMAGE. | ||||
*/ | */ | ||||
#ifndef lint | #ifndef lint | ||||
static const char rcsid[] = | static const char rcsid[] = | ||||
"$FreeBSD$"; | "$FreeBSD$"; | ||||
#endif /* not lint */ | #endif /* not lint */ | ||||
#include <ctype.h> | |||||
#include <err.h> | |||||
#include <fcntl.h> | #include <fcntl.h> | ||||
#include <string.h> | #include <string.h> | ||||
#include <stdarg.h> | #include <stdarg.h> | ||||
#include "pw.h" | #include "pw.h" | ||||
static FILE *logfile = NULL; | static FILE *logfile = NULL; | ||||
void | void | ||||
pw_log(struct userconf * cnf, int mode, int which, char const * fmt,...) | pw_log(struct userconf * cnf, int mode, int which, char const * fmt,...) | ||||
{ | { | ||||
if (cnf->logfile && *cnf->logfile) { | if (cnf->logfile && *cnf->logfile) { | ||||
if (logfile == NULL) { /* With umask==0 we need to control file access modes on create */ | if (logfile == NULL) { /* With umask==0 we need to control file access modes on create */ | ||||
int fd = open(cnf->logfile, O_WRONLY | O_CREAT | O_APPEND, 0600); | int fd = open(cnf->logfile, O_WRONLY | O_CREAT | O_APPEND, 0600); | ||||
if (fd != -1) | if (fd != -1) | ||||
logfile = fdopen(fd, "a"); | logfile = fdopen(fd, "a"); | ||||
} | } | ||||
if (logfile != NULL) { | if (logfile != NULL) { | ||||
va_list argp; | va_list argp; | ||||
time_t now = time(NULL); | time_t now = time(NULL); | ||||
struct tm *t = localtime(&now); | const char *cp; | ||||
char nfmt[256]; | |||||
const char *name; | const char *name; | ||||
struct tm *t = localtime(&now); | |||||
int i, rlen; | |||||
markj: The indentation looks wonky here, but that may just be phabricator. | |||||
truckmanAuthorUnsubmitted Not Done Inline ActionsI tried to match the existing style here with outdented "*" for pointers. truckman: I tried to match the existing style here with outdented "*" for pointers. | |||||
char nfmt[256], sname[32]; | |||||
if ((name = getenv("LOGNAME")) == NULL && (name = getenv("USER")) == NULL) | if ((name = getenv("LOGNAME")) == NULL && | ||||
name = "unknown"; | (name = getenv("USER")) == NULL) { | ||||
strcpy(sname, "unknown"); | |||||
} else { | |||||
for (i = 0, cp = name; | |||||
*cp != '\0' && | |||||
i < (int)sizeof(sname) - 1; ) { | |||||
markjUnsubmitted Not Done Inline ActionsThe cast seems superfluous since we're comparing with a constant whose value is clearly representable with an int. Does clang/coverity warn about this? (ditto below) markj: The cast seems superfluous since we're comparing with a constant whose value is clearly… | |||||
truckmanAuthorUnsubmitted Not Done Inline ActionsYes, clang complains, which I agree is bogus in this case: cc -O2 -pipe -g -MD -MF.depend.pw_log.o -MTpw_log.o -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c /usr/src/usr.sbin/pw/pw_log.c -o pw_log.o signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare] i < sizeof(sname) - 1; ) { ~ ^ ~~~~~~~~~~~~~~~~~ 1 error generated. truckman: Yes, clang complains, which I agree is bogus in this case:
cc -O2 -pipe -g -MD -MF.depend. | |||||
if (isspace(*cp)) { | |||||
cp++; | |||||
markjUnsubmitted Not Done Inline ActionsWhy not have cp++ in the loop header? markj: Why not have cp++ in the loop header? | |||||
truckmanAuthorUnsubmitted Not Done Inline ActionsGood idea. truckman: Good idea. | |||||
} else if (*cp == '%') { | |||||
if (i < (int)sizeof(sname) - 2) { | |||||
sname[i++] = '%'; | |||||
sname[i++] = '%'; | |||||
} else { | |||||
break; | |||||
} | |||||
cp++; | |||||
} else { | |||||
sname[i++] = *cp++; | |||||
} | |||||
} | |||||
if (i == 0) { | |||||
strcpy(sname, "unknown"); | |||||
} else { | |||||
sname[i] = '\0'; | |||||
} | |||||
} | |||||
/* ISO 8601 International Standard Date format */ | /* ISO 8601 International Standard Date format */ | ||||
strftime(nfmt, sizeof nfmt, "%Y-%m-%d %T ", t); | strftime(nfmt, sizeof nfmt, "%Y-%m-%d %T ", t); | ||||
sprintf(nfmt + strlen(nfmt), "[%s:%s%s] %s\n", name, Which[which], Modes[mode], fmt); | rlen = sizeof(nfmt) - strlen(nfmt); | ||||
if (rlen <= 0 || snprintf(nfmt + strlen(nfmt), rlen, | |||||
"[%s:%s%s] %s\n", sname, Which[which], Modes[mode], | |||||
fmt) >= rlen) { | |||||
warnx("log format overflow"); | |||||
} else { | |||||
va_start(argp, fmt); | va_start(argp, fmt); | ||||
vfprintf(logfile, nfmt, argp); | vfprintf(logfile, nfmt, argp); | ||||
va_end(argp); | va_end(argp); | ||||
fflush(logfile); | fflush(logfile); | ||||
} | |||||
} | } | ||||
} | } | ||||
} | } |
The indentation looks wonky here, but that may just be phabricator.