Differential D1548 Diff 5195 usr.sbin/newsyslog/newsyslog.c

Changeset View

Standalone View

usr.sbin/newsyslog/newsyslog.c

/*-		/*-
* ------+---------+---------+-------- + --------+---------+---------+---------*		* ------+---------+---------+-------- + --------+---------+---------+---------*
* This file includes significant modifications done by:		* This file includes significant modifications done by:
* Copyright (c) 2003, 2004 - Garance Alistair Drosehn <gad@FreeBSD.org>.		* Copyright (c) 2003, 2004 - Garance Alistair Drosehn <gad@FreeBSD.org>.
		* Copyright (c) 2014, 2015 Allan Jude <allanjude@FreeBSD.org>.
* All rights reserved.		* All rights reserved.
*		*
* Redistribution and use in source and binary forms, with or without		* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions		* modification, are permitted provided that the following conditions
* are met:		* are met:
* 1. Redistributions of source code must retain the above copyright		* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.		* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright		* 2. Redistributions in binary form must reproduce the above copyright
▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Lines
#include <paths.h>		#include <paths.h>
#include <pwd.h>		#include <pwd.h>
#include <signal.h>		#include <signal.h>
#include <stdio.h>		#include <stdio.h>
#include <libgen.h>		#include <libgen.h>
#include <stdlib.h>		#include <stdlib.h>
#include <string.h>		#include <string.h>
#include <time.h>		#include <time.h>
		#include <ucl.h>
#include <unistd.h>		#include <unistd.h>

#include "pathnames.h"		#include "pathnames.h"
#include "extern.h"		#include "extern.h"

/*		/*
* Compression suffixes		* Compression suffixes
*/		*/
▲ Show 20 Lines • Show All 109 Lines • ▼ Show 20 Lines	struct oldlog_entry {
char fname; / Filename of the log file */		char fname; / Filename of the log file */
time_t t; /* Parsed timestamp of the logfile */		time_t t; /* Parsed timestamp of the logfile */
};		};

typedef enum {		typedef enum {
FREE_ENT, KEEP_ENT		FREE_ENT, KEEP_ENT
} fk_entry;		} fk_entry;

		/**
		* Config parser helper callback function pointer alias
		*/
		typedef bool (conf_helper_func)(const ucl_object_t , const ucl_object_t *,
		cse_cem_gmail_comUnsubmitted Done Inline Actions Don't need the backslash. `typedef` isn't part of the preprocessor. cse_cem_gmail_com: Don't need the backslash. `typedef` isn't part of the preprocessor.
		void *);
		cse_cem_gmail_comUnsubmitted Done Inline Actions It looks like the 3rd parameter here should be `struct conf_entry ` based on later functions. cse_cem_gmail_com:* It looks like the 3rd parameter here should be `struct conf_entry *` based on later functions.
		allanjudeAuthorUnsubmitted Done Inline Actions I originally had this code in libucl, so it was generalized to work in places other than just newsyslog. I just dumped it into newsyslog for now. allanjude: I originally had this code in libucl, so it was generalized to work in places other than just…

		/**
		* Config parser helper callback map. Maps UCL keys to callback functions that
		* parse the key and store the value in the correct struct member
		*/
		typedef struct conf_helper_callback_map {
		const char *conf_key;
		conf_helper_func callback;
		} conf_helper_map;

STAILQ_HEAD(cflist, conf_entry);		STAILQ_HEAD(cflist, conf_entry);
static SLIST_HEAD(swlisthead, sigwork_entry) swhead =		static SLIST_HEAD(swlisthead, sigwork_entry) swhead =
SLIST_HEAD_INITIALIZER(swhead);		SLIST_HEAD_INITIALIZER(swhead);
static SLIST_HEAD(zwlisthead, zipwork_entry) zwhead =		static SLIST_HEAD(zwlisthead, zipwork_entry) zwhead =
SLIST_HEAD_INITIALIZER(zwhead);		SLIST_HEAD_INITIALIZER(zwhead);
STAILQ_HEAD(ilist, include_entry);		STAILQ_HEAD(ilist, include_entry);

int dbg_at_times; /* -D Show details of 'trim_at' code */		int dbg_at_times; /* -D Show details of 'trim_at' code */
Show All 27 Lines	static char daytime[DAYTIME_LEN];/* The current time in human readable form,
* used for rotation-tracking messages. */		* used for rotation-tracking messages. */
static char hostname[MAXHOSTNAMELEN]; /* hostname */		static char hostname[MAXHOSTNAMELEN]; /* hostname */

static const char *path_syslogpid = _PATH_SYSLOGPID;		static const char *path_syslogpid = _PATH_SYSLOGPID;

static struct cflist get_worklist(char *files);		static struct cflist get_worklist(char *files);
static void parse_file(FILE cf, struct cflist work_p, struct cflist *glob_p,		static void parse_file(FILE cf, struct cflist work_p, struct cflist *glob_p,
struct conf_entry defconf_p, struct ilist inclist);		struct conf_entry defconf_p, struct ilist inclist);
		static void parse_ucl(FILE cf, struct cflist work_p, struct cflist *glob_p,
		struct conf_entry *defconf_p);
		static bool conf_set_uid(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_gid(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_mode(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_count(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_size(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_compress(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_flags(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_when(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_command(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_signal(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_pidfile(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
		static bool conf_set_unknown(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target);
static void add_to_queue(const char fname, struct ilist inclist);		static void add_to_queue(const char fname, struct ilist inclist);
static char sob(char p);		static char sob(char p);
static char son(char p);		static char son(char p);
static int isnumberstr(const char *);		static int isnumberstr(const char *);
static int isglobstr(const char *);		static int isglobstr(const char *);
static char missing_field(char p, char *errline);		static char missing_field(char p, char *errline);
static void change_attrs(const char , const struct conf_entry );		static void change_attrs(const char , const struct conf_entry );
static const char get_logfile_suffix(const char logfile);		static const char get_logfile_suffix(const char logfile);
Show All 16 Lines
static void parse_args(int argc, char **argv);		static void parse_args(int argc, char **argv);
static int parse_doption(const char *doption);		static int parse_doption(const char *doption);
static void usage(void);		static void usage(void);
static int log_trim(const char logname, const struct conf_entry log_ent);		static int log_trim(const char logname, const struct conf_entry log_ent);
static int age_old_log(const char *file);		static int age_old_log(const char *file);
static void savelog(char from, char to);		static void savelog(char from, char to);
static void createdir(const struct conf_entry ent, char dirpart);		static void createdir(const struct conf_entry ent, char dirpart);
static void createlog(const struct conf_entry *ent);		static void createlog(const struct conf_entry *ent);
		static bool conf_helper_mapper (const ucl_object_t obj, conf_helper_map map, void *target);

/*		/*
* All the following take a parameter of 'int', but expect values in the		* All the following take a parameter of 'int', but expect values in the
* range of unsigned char. Define wrappers which take values of type 'char',		* range of unsigned char. Define wrappers which take values of type 'char',
* whether signed or unsigned, and ensure they end up in the right range.		* whether signed or unsigned, and ensure they end up in the right range.
*/		*/
#define isdigitch(Anychar) isdigit((u_char)(Anychar))		#define isdigitch(Anychar) isdigit((u_char)(Anychar))
#define isprintch(Anychar) isprint((u_char)(Anychar))		#define isprintch(Anychar) isprint((u_char)(Anychar))
▲ Show 20 Lines • Show All 746 Lines • ▼ Show 20 Lines
* Parse a configuration file and update a linked list of all the logs to		* Parse a configuration file and update a linked list of all the logs to
* process.		* process.
*/		*/
static void		static void
parse_file(FILE cf, struct cflist work_p, struct cflist *glob_p,		parse_file(FILE cf, struct cflist work_p, struct cflist *glob_p,
struct conf_entry defconf_p, struct ilist inclist)		struct conf_entry defconf_p, struct ilist inclist)
{		{
char line[BUFSIZ], parse, q;		char line[BUFSIZ], parse, q;
char cp, errline, *group;		char cp, errline, group, fread;
struct conf_entry *working;		struct conf_entry *working;
struct passwd *pwd;		struct passwd *pwd;
struct group *grp;		struct group *grp;
glob_t pglob;		glob_t pglob;
int eol, ptm_opts, res, special;		int eol, ptm_opts, res, special, uclver, n;
size_t i;		size_t i;
		cse_cem_gmail_comUnsubmitted Done Inline Actions Using double for this is odd. Usually we just multiply by a power of 10 (or 0x10). cse_cem_gmail_com: Using double for this is odd. Usually we just multiply by a power of 10 (or 0x10).
		allanjudeAuthorUnsubmitted Done Inline Actions So make 'uclver' an int, and multiply the result of strtod(3) by 10? allanjude: So make 'uclver' an int, and multiply the result of strtod(3) by 10?
		cse_cem_gmail_comUnsubmitted Done Inline Actions That'd work. You could also just use an integer version (e.g. 1) and `strtol` or whatever. (This is just versioning the config schema, right?) cse_cem_gmail_com: That'd work. You could also just use an integer version (e.g. 1) and `strtol` or whatever.
		eadlerUnsubmitted Done Inline Actions style(9) prefers not having init inline eadler: style(9) prefers not having init inline

errline = NULL;		errline = NULL;
		fread = fgets(line, BUFSIZ, cf);
		cse_cem_gmail_comUnsubmitted Done Inline Actions Check the return value of `fgets(3)` before using `line`. cse_cem_gmail_com: Check the return value of `fgets(3)` before using `line`.
		if (fread == NULL) {
		allanjudeAuthorUnsubmitted Not Done Inline Actions taste the first line of the file. If it isn't what we want, rewind() and continue as before Otherwise, pass it off to the new config parser allanjude: taste the first line of the file. If it isn't what we want, rewind() and continue as before…
		errx(1, "Failed to read config file. Error #%i\n", errno);
		}
		rewind(cf);
		if (strncmp(line, "#fucl", 5) == 0) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions You can drop the `lineptr` variable entirely. You would use `strtod` like: `uclver = strtod(&line[5], NULL);` cse_cem_gmail_com: You can drop the `lineptr` variable entirely. You would use `strtod` like: `uclver = strtod…
		allanjudeAuthorUnsubmitted Done Inline Actions dropped allanjude: dropped
		/* The config file is a FreeBSD UCL */
		cse_cem_gmail_comUnsubmitted Done Inline Actions style(9): The body of the `if()` statement goes on a line by itself. cse_cem_gmail_com: style(9): The body of the `if()` statement goes on a line by itself.
		/* Determine its schema version */
		if (strlen(line) < 7)
		eadlerUnsubmitted Done Inline Actions theoretical overflow not handled, but is likely fine eadler: theoretical overflow not handled, but is likely fine
		errx(1, "Error: Invalid UCL header: %s\n", line);
		n = sscanf(line + 5, "%i", &uclver);
		cse_cem_gmail_comUnsubmitted Done Inline Actions Floating point equality is somewhat fragile. Use integers for this. cse_cem_gmail_com: Floating point equality is somewhat fragile. Use integers for this.
		if (n == 0 \|\| uclver == 0)
		errx(1, "Error: UCL version number is invalid: '%s'\n", line + 5);
		if (verbose)
		printf("\tUCL version is %i\n", uclver);
		if (uclver > 1)
		errx(1, "UCL version too new\n");
		else if (uclver == 1)
		parse_ucl(cf, work_p, glob_p, defconf_p);
		return;
		}
while (fgets(line, BUFSIZ, cf)) {		while (fgets(line, BUFSIZ, cf)) {
if ((line[0] == '\n') \|\| (line[0] == '#') \|\|		if ((line[0] == '\n') \|\| (line[0] == '#') \|\|
(strlen(line) == 0))		(strlen(line) == 0))
continue;		continue;
if (errline != NULL)		if (errline != NULL)
free(errline);		free(errline);
errline = strdup(line);		errline = strdup(line);
for (cp = line + 1; *cp != '\0'; cp++) {		for (cp = line + 1; *cp != '\0'; cp++) {
▲ Show 20 Lines • Show All 350 Lines • ▼ Show 20 Lines	no_trimat:
} else {		} else {
STAILQ_INSERT_TAIL(work_p, working, cf_nextp);		STAILQ_INSERT_TAIL(work_p, working, cf_nextp);
}		}
}		}
if (errline != NULL)		if (errline != NULL)
free(errline);		free(errline);
}		}

		/*
		* Parse a UCL configuration file and update a linked list of all the logs to
		* process.
		*/
		static void
		parse_ucl(FILE cf, struct cflist work_p, struct cflist *glob_p,
		struct conf_entry *defconf_p)
		{
		struct ucl_parser *parser = NULL;
		ucl_object_t *conf_root = NULL;
		ucl_object_iter_t it_conf = NULL;
		const ucl_object_t obj_file = NULL, obj_key = NULL;
		const char cf_key = NULL, cf_filename;
		struct conf_entry *working;
		bool success = false;
		conf_helper_map map[] =
		{
		allanjudeAuthorUnsubmitted Not Done Inline Actions This is the map that uses the callbacks. It ended up different than I originally pictures, but I think it is still nicer than a giant strcmp() if ladder allanjude: This is the map that uses the callbacks. It ended up different than I originally pictures, but…
		{ "file", NULL }, /* The file is set when the conf_entry is created */
		{ "user", (conf_helper_func) conf_set_uid },
		{ "group", (conf_helper_func) conf_set_gid },
		{ "mode", (conf_helper_func) conf_set_mode },
		{ "count", (conf_helper_func) conf_set_count },
		{ "size", (conf_helper_func) conf_set_size },
		{ "compress", (conf_helper_func) conf_set_compress },
		{ "flags", (conf_helper_func) conf_set_flags },
		{ "when", (conf_helper_func) conf_set_when },
		{ "command", (conf_helper_func) conf_set_command },
		{ "signal", (conf_helper_func) conf_set_signal },
		{ "pidfile", (conf_helper_func) conf_set_pidfile },
		{ NULL, (conf_helper_func) conf_set_unknown }
		cse_cem_gmail_comUnsubmitted Done Inline Actions I think you can drop the `(conf_helper_func)` casts if you change the function typedef from `void ` to `struct conf_entry `. cse_cem_gmail_com: I think you can drop the `(conf_helper_func)` casts if you change the function typedef from…
		allanjudeAuthorUnsubmitted Done Inline Actions see response to https://reviews.freebsd.org/D1548#inline-9104 allanjude: see response to https://reviews.freebsd.org/D1548#inline-9104
		};

		parser = ucl_parser_new(UCL_PARSER_KEY_LOWERCASE);
		ucl_parser_add_fd(parser, fileno(cf));
		cse_cem_gmail_comUnsubmitted Done Inline Actions You probably want to `lseek(2)` the fd back to the beginning — `fgets(3)` and other stdio functions may buffer large chunks of the file and the fd may be well past the beginning of the UCL stuff. cse_cem_gmail_com: You probably want to `lseek(2)` the fd back to the beginning — `fgets(3)` and other stdio…
		allanjudeAuthorUnsubmitted Done Inline Actions I moved the rewind(cf) that was done only in the case of a non-ucl config earlier, so it should resolve this allanjude: I moved the rewind(cf) that was done only in the case of a non-ucl config earlier, so it should…
		if (ucl_parser_get_error(parser)) {
		errx(1, "UCL Error occured: %s\n",
		ucl_parser_get_error(parser));
		}
		conf_root = ucl_parser_get_object(parser);
		if (ucl_parser_get_error(parser)) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions extra space (style) cse_cem_gmail_com: extra space (style)
		errx(1, "Error: Parse Error occured: %s\n",
		ucl_parser_get_error(parser));
		}

		it_conf = ucl_object_iterate_new(conf_root);
		while ((obj_file = ucl_object_iterate_safe(it_conf, true)) != NULL) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions extra space cse_cem_gmail_com: extra space
		cf_key = ucl_object_key(obj_file);
		if (strcasecmp(cf_key, "default") == 0) {
		/* XXX: TODO: Need special handling of the 'default' entry */
		eadlerUnsubmitted Not Done Inline Actions TODO comment :) eadler: TODO comment :)
		continue;
		}
		if (verbose > 2)
		printf("File: %s\n", cf_key);

		cse_cem_gmail_comUnsubmitted Done Inline Actions There's lots of this, so I'm not going to mark it every time. But, in style(9) it is suggested that `if`/`while`/`for` braces be omitted when they are not required (with exceptions for complicated bodies). `printf` is not complicated. cse_cem_gmail_com: There's lots of this, so I'm not going to mark it every time. But, in style(9) it is suggested…
		/* Find the filename key inside this object */
		obj_key = ucl_object_find_key(obj_file, "file");
		if (obj_key == NULL) {
		if (verbose > 2)
		printf("Error: Unable to find filename in key: "
		"%s\n", cf_key);
		continue;
		}
		cf_filename = ucl_object_tostring(obj_key);

		working = init_entry(cf_filename, defconf_p);
		working->flags = 0;
		working->compress = COMPRESS_NONE;
		working->pid_cmd_file = NULL;
		working->sig = SIGHUP;

		success = conf_helper_mapper(obj_file, map, working);
		if (!success) {
		eadlerUnsubmitted Done Inline Actions if (!success) {} ? eadler: if (!success) {} ?
		warnx("Config parsing issue in section %s (%s)\n",
		cf_key, cf_filename);
		free_entry(working);
		continue;
		}

		/*
		* Finish figuring out what pid-file to use (if any) in
		* later processing if this logfile needs to be rotated.
		*/
		if ((working->flags & CE_NOSIGNAL) != 0) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions The usual pattern for this is: `if ((working->flags & CE_NOSIGNAL) != 0) {` cse_cem_gmail_com: The usual pattern for this is: `if ((working->flags & CE_NOSIGNAL) != 0) {`
		allanjudeAuthorUnsubmitted Done Inline Actions Not my code, but since it is inside my function, fixing it allanjude: Not my code, but since it is inside my function, fixing it
		/*
		* This config-entry specified 'n' for nosignal,
		* see if it also specified an explicit pid_cmd_file.
		* This would be a pretty pointless combination.
		*/
		if (working->pid_cmd_file != NULL) {
		warnx("Ignoring '%s' because flag 'n' was specified in section '%s'\n",
		working->pid_cmd_file, cf_key);
		free(working->pid_cmd_file);
		working->pid_cmd_file = NULL;
		}
		} else if (working->pid_cmd_file == NULL) {
		/*
		* This entry did not specify the 'n' flag, which
		* means it should signal syslogd unless it had
		* specified some other pid-file (and obviously the
		* syslog pid-file will not be for a process-group).
		* Also, we should only try to notify syslog if we
		* are root.
		*/
		if (working->flags & CE_SIGNALGROUP) {
		warnx("Ignoring flag 'U' in section '%s'\n",
		cf_key);
		working->flags &= ~CE_SIGNALGROUP;
		}
		if (needroot)
		working->pid_cmd_file = strdup(path_syslogpid);
		}
		/*
		* Add this entry to the appropriate list of entries, unless
		* it was some kind of special entry (eg: <default>).
		*/
		if (working->flags & CE_GLOB) {
		STAILQ_INSERT_TAIL(glob_p, working, cf_nextp);
		} else {
		STAILQ_INSERT_TAIL(work_p, working, cf_nextp);
		}
		}
		ucl_object_iterate_free(it_conf);

		if (parser != NULL)
		ucl_parser_free(parser);
		if (conf_root != NULL)
		ucl_object_unref(conf_root);
		}

		bool
		conf_set_uid(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		struct passwd *pwd;
		const char *val = NULL;

		if (ucl_object_type(obj) == UCL_INT) {
		target->uid = ucl_object_toint(obj);
		return (true);
		} else {
		cse_cem_gmail_comUnsubmitted Done Inline Actions You can drop `result` entirely. `target->uid = ucl_object_toint(obj);` cse_cem_gmail_com: You can drop `result` entirely. `target->uid = ucl_object_toint(obj);`
		allanjudeAuthorUnsubmitted Done Inline Actions ok allanjude: ok
		val = ucl_object_tostring_forced(obj);
		if ((pwd = getpwnam(val)) == NULL) {
		warnx("error in config file; unknown user: '%s' in section '%s'\n",
		val, ucl_object_key(top));
		return (false);
		cse_cem_gmail_comUnsubmitted Done Inline Actions Use `atol` or `strtol`. `uid_t` is unsigned and yes, somebody has >2 billion uids (or at least uses the high numbers). cse_cem_gmail_com: Use `atol` or `strtol`. `uid_t` is unsigned and yes, somebody has >2 billion uids (or at least…
		allanjudeAuthorUnsubmitted Done Inline Actions this case doesn't need to be here, it is left over. If the user entered a number, it will by type UCL_INT, unless they put a decimal in it or something silly, in which case they deserve an error anyway. allanjude: this case doesn't need to be here, it is left over. If the user entered a number, it will by…
		}
		cse_cem_gmail_comUnsubmitted Done Inline Actions `target->uid = atol(val);` or similar cse_cem_gmail_com: `target->uid = atol(val);` or similar
		target->uid = pwd->pw_uid;
		return (true);
		}
		warnx("error in config file; invalid value for user in section '%s'\n",
		ucl_object_key(obj));
		return (false);
		}

		bool
		conf_set_gid(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		struct group *grp;
		cse_cem_gmail_comUnsubmitted Done Inline Actions `target->uid = pwd->pw_uid;` cse_cem_gmail_com: `target->uid = pwd->pw_uid;`
		const char *val = NULL;

		if (ucl_object_type(obj) == UCL_INT) {
		target->gid = ucl_object_toint(obj);
		return (true);
		} else {
		val = ucl_object_tostring_forced(obj);
		if ((grp = getgrnam(val)) == NULL) {
		warnx("error in config file; unknown group '%s' in section '%s'\n",
		val, ucl_object_key(top));
		return (false);
		}
		target->gid = grp->gr_gid;
		return (true);
		}
		warnx("error in config file; invalid value for group in section '%s'\n",
		cse_cem_gmail_comUnsubmitted Done Inline Actions extra parens, you don't need them around function calls cse_cem_gmail_com: extra parens, you don't need them around function calls
		allanjudeAuthorUnsubmitted Done Inline Actions those were existing, but removed allanjude: those were existing, but removed
		ucl_object_key(obj));
		return (false);
		}

		bool
		conf_set_mode(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const char *val = NULL;
		cse_cem_gmail_comUnsubmitted Done Inline Actions Probably `atol` here too. cse_cem_gmail_com: Probably `atol` here too.

		val = ucl_object_tostring_forced(obj);
		if (sscanf(val, "%o", &target->permissions) != 1) {
		errx(1, "error in config file; bad permissions '%s' in section '%s'\n",
		val, ucl_object_key(top));
		return (false);
		}
		cse_cem_gmail_comUnsubmitted Done Inline Actions style(9): `return (true);` cse_cem_gmail_com: style(9): `return (true);`
		return (true);
		}

		bool
		conf_set_count(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		cse_cem_gmail_comUnsubmitted Done Inline Actions Probably: `if (sscanf(...) != 1) {`. sscanf may return EOF (-1) as well as (0) depending on what went wrong. Anything other than 1 conversion is failure for us. cse_cem_gmail_com: Probably: `if (sscanf(...) != 1) {`. sscanf may return EOF (-1) as well as (0) depending on…
		{
		const char *val = NULL;

		val = ucl_object_tostring_forced(obj);
		cse_cem_gmail_comUnsubmitted Done Inline Actions Ah, I see, you force octal here. Ok. cse_cem_gmail_com: Ah, I see, you force octal here. Ok.
		if (sscanf(val, "%d", &target->numlogs) != 1 \|\| target->numlogs < 0) {
		errx(1, "error in config file; bad value '%s' for count of logs to save in section '%s'\n",
		val, ucl_object_key(top));
		}
		return (true);
		}

		bool
		conf_set_size(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		cse_cem_gmail_comUnsubmitted Done Inline Actions (same re: `sscanf(...) != 1`.) cse_cem_gmail_com: (same re: `sscanf(...) != 1`.)
		const char *val = NULL;
		int log_size;

		val = ucl_object_tostring_forced(obj);
		if (isdigitch(*val)) {
		log_size = ucl_object_toint(obj);
		target->trsize = kbytes(log_size);
		} else if (strcmp(val, "*") == 0) {
		target->trsize = -1;
		} else {
		warnx("Invalid value of '%s' for 'size' in section '%s'\n",
		val, ucl_object_key(top));
		target->trsize = -1;
		return (false);
		allanjudeAuthorUnsubmitted Not Done Inline Actions This is mostly just verbatim from the original code. It can likely be improved, but I wanted to try to avoid introducing too many new bugs at once allanjude: This is mostly just verbatim from the original code. It can likely be improved, but I wanted to…
		}
		return (true);
		}

		cse_cem_gmail_comUnsubmitted Done Inline Actions These on the same line too cse_cem_gmail_com: These on the same line too
		bool
		conf_set_compress(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const char *val = NULL;

		val = ucl_object_tostring_forced(obj);
		if (strcasecmp(val, "j") == 0 \|\| strncasecmp(val, "bz", 2) == 0) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions these should be on the same line (style(9)) cse_cem_gmail_com: these should be on the same line (style(9))
		target->compress = COMPRESS_BZIP2;
		} else if (strcasecmp(val, "x") == 0 \|\|
		strncasecmp(val, "xz", 2) == 0 \|\|
		strcasecmp(val, "yes") == 0) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions Maybe just `strcasecmp()` for "yes"? cse_cem_gmail_com: Maybe just `strcasecmp()` for "yes"?
		target->compress = COMPRESS_XZ;
		} else if (strcasecmp(val, "z") == 0 \|\| strncasecmp(val, "gz", 2) == 0) {
		target->compress = COMPRESS_GZIP;
		} else if (strlen(val) == 0 \|\| strncasecmp(val, "no", 2) == 0) {
		target->compress = COMPRESS_NONE;
		} else {
		warnx("Invalid flag '%s' in section '%s'\n",
		val, ucl_object_key(top));
		cse_cem_gmail_comUnsubmitted Done Inline Actions No 'n' needed for xz, I think? cse_cem_gmail_com: No 'n' needed for xz, I think?
		allanjudeAuthorUnsubmitted Done Inline Actions It is done so it allows the user to do 'xzip' as well. same reason bzip is cut to 4 chars, so bzip or bzip2 both are accepted. allanjude: It is done so it allows the user to do 'xzip' as well. same reason bzip is cut to 4 chars, so…
		cse_cem_gmail_comUnsubmitted Done Inline Actions Ah, I got bzip/bzip2 but hadn't seen xzip before. Seems reasonable to me. cse_cem_gmail_com: Ah, I got bzip/bzip2 but hadn't seen xzip before. Seems reasonable to me.
		allanjudeAuthorUnsubmitted Done Inline Actions This question made me think about it more, and improve it further use strncasecmp(val, ..., 2) in all cases: so gzip accepts: gz gzip bzip2: bz bz2 bzip bzip2 xz: xz xzip yes none: no none (added yes to xz since we accept no) allanjude: This question made me think about it more, and improve it further use strncasecmp(val, ..., 2)…
		return (false);
		}
		return (true);
		}

		bool
		conf_set_flags(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const ucl_object_t *obj_flag = NULL;
		ucl_object_iter_t it_flag = NULL;
		const char *val = NULL;

		it_flag = ucl_object_iterate_new(obj);
		while ((obj_flag = ucl_object_iterate_safe(it_flag, true)) != NULL) {
		val = ucl_object_tostring(obj_flag);
		if (verbose > 2)
		printf("\t\tFound Flag: %s\n", val);
		if (val == NULL) {
		warnx("Invalid flag in section '%s'\n",
		ucl_object_key(top));
		continue;
		}
		cse_cem_gmail_comUnsubmitted Done Inline Actions extra space cse_cem_gmail_com: extra space
		if (strcasecmp(val, "b") == 0 \|\| strcasecmp(val, "binary") == 0) {
		target->flags \|= CE_BINARY;
		} else if (strcasecmp(val, "c") == 0 \|\| strcasecmp(val, "create") == 0) {
		target->flags \|= CE_CREATE;
		} else if (strcasecmp(val, "d") == 0 \|\| strcasecmp(val, "nodump") == 0) {
		target->flags \|= CE_NODUMP;
		} else if (strcasecmp(val, "g") == 0 \|\| strcasecmp(val, "glob") == 0) {
		target->flags \|= CE_GLOB;
		} else if (strcasecmp(val, "j") == 0 \|\| strncasecmp(val, "bz", 2) == 0) {
		target->compress = COMPRESS_BZIP2;
		} else if (strcasecmp(val, "n") == 0 \|\| strcasecmp(val, "nosignal") == 0) {
		target->flags \|= CE_NOSIGNAL;
		} else if (strcasecmp(val, "r") == 0 \|\| strcasecmp(val, "command") == 0) {
		target->flags \|= CE_PID2CMD;
		} else if (strcasecmp(val, "u") == 0 \|\| strcasecmp(val, "signalgroup") == 0) {
		target->flags \|= CE_SIGNALGROUP;
		} else if (strcasecmp(val, "w") == 0) {
		/* Depreciated flag - keep for compatibility purposes */
		} else if (strcasecmp(val, "x") == 0 \|\| strncasecmp(val, "xz", 2) == 0) {
		target->compress = COMPRESS_XZ;
		} else if (strcasecmp(val, "z") == 0 \|\| strncasecmp(val, "gz", 2) == 0) {
		target->compress = COMPRESS_GZIP;
		} else {
		/**
		* f: Used by OpenBSD for "CE_FOLLOW"
		* m: Used by OpenBSD for "CE_MONITOR"
		* p: Used by NetBSD for "CE_PLAIN0"
		*/
		errx(1, "illegal flag '%s' in section '%s'\n",
		val, ucl_object_key(top));
		}
		}
		ucl_object_iterate_free(it_flag);
		if (verbose > 3)
		printf("\t\tFinal Flags: %i\n", target->flags);
		return (true);
		}

		bool
		conf_set_when(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		int ptm_opts, res;
		char *ep;
		u_long ul;
		const char *val;

		val = ucl_object_tostring_forced(obj);
		errno = 0;
		eadlerUnsubmitted Done Inline Actions if ul == 0, check errno? eadler: if ul == 0, check errno?
		ul = strtoul(val, &ep, 10);
		if (errno)
		errx(1, "interval '%ld' is invalid in section '%s'\n",
		ul, ucl_object_key(top));
		if (ep == val)
		target->hours = 0;
		else if (ep == '')
		target->hours = -1;
		else if (ul > INT_MAX)
		errx(1, "interval '%ld' is too large in section '%s'\n",
		ul, ucl_object_key(top));
		else
		cse_cem_gmail_comUnsubmitted Done Inline Actions same line cse_cem_gmail_com: same line
		target->hours = ul;

		if (ep == '\0' \|\| strcmp(ep, "") == 0)
		cse_cem_gmail_comUnsubmitted Done Inline Actions also same line cse_cem_gmail_com: also same line
		return (true);
		if (ep != '@' && ep != '$')
		errx(1, "malformed interval '%s' in section '%s'\n",
		val, ucl_object_key(top));
		cse_cem_gmail_comUnsubmitted Done Inline Actions same cse_cem_gmail_com: same

		target->flags \|= CE_TRIMAT;
		target->trim_at = ptime_init(NULL);
		ptm_opts = PTM_PARSE_ISO8601;
		if (*ep == '$') {
		ptm_opts = PTM_PARSE_DWM;
		}
		ptm_opts \|= PTM_PARSE_MATCHDOM;
		res = ptime_relparse(target->trim_at, ptm_opts,
		ptimeget_secs(timenow), ep + 1);
		cse_cem_gmail_comUnsubmitted Done Inline Actions braces can be dropped here. cse_cem_gmail_com: braces can be dropped here.
		if (res == -2)
		errx(1, "nonexistent time for 'at' value '%s' in section '%s'\n",
		val, ucl_object_key(top));
		else if (res < 0)
		errx(1, "malformed 'at' value '%s' in section '%s'\n",
		val, ucl_object_key(top));
		return (true);
		}

		bool
		conf_set_command(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const char *val = NULL;

		val = ucl_object_tostring(obj);
		cse_cem_gmail_comUnsubmitted Done Inline Actions same cse_cem_gmail_com: same
		target->pid_cmd_file = strdup(val);
		if (verbose > 3)
		printf("\t\tSetting command: '%s' in section %s\n", val,
		ucl_object_key(top));
		return (true);
		}

		bool
		conf_set_signal(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const char *val = NULL;

		val = ucl_object_tostring(obj);
		if (isdigit(*val)) {
		target->sig = atoi(val);
		} else {
		/* XXX: TODO: Add signal name mapping */
		errx(1,
		"illegal signal number '%s' in section '%s'\n",
		val, ucl_object_key(top));
		}
		if (target->sig < 1 \|\| target->sig >= NSIG) {
		errx(1,
		"illegal signal number '%s' in section '%s'\n",
		val, ucl_object_key(top));
		}
		return (true);
		}

		bool
		conf_set_pidfile(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target)
		{
		const char *val = NULL;

		val = ucl_object_tostring_forced(obj);
		if (*val == '/') {
		target->pid_cmd_file = strdup(val);
		} else {
		errx(1, "illegal pid file '%s' in section '%s'\n",
		val, ucl_object_key(top));
		}
		return (true);
		}

		bool
		conf_set_unknown(const ucl_object_t top, const ucl_object_t obj, struct conf_entry *target __unused)
		{
		warnx("Unknown key '%s' in section '%s'\n",
		ucl_object_key(obj), ucl_object_key(top));
		return (false);
		}

static char *		static char *
missing_field(char p, char errline)		missing_field(char p, char errline)
{		{

if (!p \|\| !*p)		if (!p \|\| !*p)
errx(1, "missing field in config file:\n%s", errline);		errx(1, "missing field in config file:\n%s", errline);
return (p);		return (p);
}		}
		cse_cem_gmail_comUnsubmitted Done Inline Actions You can drop the comment, the pattern is well understood. Alternatively, you can include `sys/cdefs.h` (if it isn't already) and use `__unused` like: `conf_set_unknown(..., struct conf_entry target __unused)` cse_cem_gmail_com:* You can drop the comment, the pattern is well understood. Alternatively, you can include…

/*		/*
* In our sort we return it in the reverse of what qsort normally		* In our sort we return it in the reverse of what qsort normally
* would do, as we want the newest files first. If we have two		* would do, as we want the newest files first. If we have two
* entries with the same time we don't really care about order.		* entries with the same time we don't really care about order.
*		*
* Support function for qsort() in delete_oldest_timelog().		* Support function for qsort() in delete_oldest_timelog().
*/		*/
▲ Show 20 Lines • Show All 1,220 Lines • ▼ Show 20 Lines	if (failed)
warn("can't chown %s", fname);		warn("can't chown %s", fname);
}		}

if (ent->flags & CE_NODUMP) {		if (ent->flags & CE_NODUMP) {
failed = chflags(fname, UF_NODUMP);		failed = chflags(fname, UF_NODUMP);
if (failed)		if (failed)
warn("can't chflags %s NODUMP", fname);		warn("can't chflags %s NODUMP", fname);
}		}
		}

		bool
		conf_helper_mapper(const ucl_object_t obj, conf_helper_map map, void *target)
		cse_cem_gmail_comUnsubmitted Done Inline Actions extra space cse_cem_gmail_com: extra space
		{
		ucl_object_iter_t it;
		const ucl_object_t *cur;
		const char *key = NULL;
		int i;
		bool ret = true, found = false;

		it = ucl_object_iterate_new(obj);
		while ((cur = ucl_object_iterate_safe(it, true)) != NULL && ret) {
		key = ucl_object_key(cur);
		found = false;
		for (i = 0; map[i].conf_key; i++) {
		if (strcasecmp(map[i].conf_key, key) != 0)
		cse_cem_gmail_comUnsubmitted Done Inline Actions If you want to reduce indentation, you can do: if (strcasecmp(...) != 0) continue; And then the rest of the loop body can be unindented. cse_cem_gmail_com: If you want to reduce indentation, you can do: if (strcasecmp(...) != 0) continue; And…
		continue;
		found = true;
		if (map[i].callback != NULL) {
		ret = map[i].callback(obj, cur, target);
		cse_cem_gmail_comUnsubmitted Done Inline Actions Can drop NULL check entirely. Just don't put a NULL callback in your table :-). cse_cem_gmail_com: Can drop NULL check entirely. Just don't put a NULL callback in your table :-).
		allanjudeAuthorUnsubmitted Done Inline Actions This is there on purpose. We have a NULL callback for the 'file' key, because it is handled elsewhere, and we don't want that key to trip the 'default' callback. See line #1486 This should be documented better, my goal is to make something generic to speed up the process of converting other utilities to UCL. allanjude: This is there on purpose. We have a NULL callback for the 'file' key, because it is handled…
		cse_cem_gmail_comUnsubmitted Done Inline Actions Ok. cse_cem_gmail_com: Ok.
		}
		break;
		}
		if (!found && map[i].callback != NULL) {
		cse_cem_gmail_comUnsubmitted Done Inline Actions `if (!found && ...)` cse_cem_gmail_com: `if (!found && ...)`
		cse_cem_gmail_comUnsubmitted Done Inline Actions Typo — I think you want `!found`? cse_cem_gmail_com: Typo — I think you want `!found`?
		allanjudeAuthorUnsubmitted Done Inline Actions good catch, thank you allanjude: good catch, thank you
		/* Call default handler if there is one */
		ret = map[i].callback(obj, cur, target);
		}
		}
		ucl_object_iterate_free(it);
		return (ret);
		cse_cem_gmail_comUnsubmitted Done Inline Actions (ret) cse_cem_gmail_com: (ret)
}		}