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_com: Don't need the backslash. `typedef` isn't part of the preprocessor. | |||||
void *); | |||||
Done Inline ActionsIt 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. | |||||
Done Inline ActionsI 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; | ||||
int uclver = 0; | |||||
Done Inline ActionsUsing 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). | |||||
Done Inline ActionsSo 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? | |||||
Done Inline ActionsThat'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. | |||||
Done Inline Actionsstyle(9) prefers not having init inline eadler: style(9) prefers not having init inline | |||||
size_t i; | size_t i; | ||||
errline = NULL; | errline = NULL; | ||||
fread = fgets(line, BUFSIZ, cf); | |||||
Done Inline ActionsCheck 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) { | |||||
Not Done Inline Actionstaste 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) { | |||||
Done Inline ActionsYou 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… | |||||
Done Inline Actionsdropped allanjude: dropped | |||||
/* The config file is a FreeBSD UCL */ | |||||
Done Inline Actionsstyle(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 */ | |||||
uclver = strtol(&line[5], NULL, 10); | |||||
Done Inline Actionstheoretical overflow not handled, but is likely fine eadler: theoretical overflow not handled, but is likely fine | |||||
if (verbose) | |||||
printf("\tUCL Version is %i\n", uclver); | |||||
Done Inline ActionsFloating point equality is somewhat fragile. Use integers for this. cse_cem_gmail_com: Floating point equality is somewhat fragile. Use integers for this. | |||||
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[] = | |||||
{ | |||||
Not Done Inline ActionsThis 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 } | |||||
Done Inline ActionsI 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… | |||||
Done Inline Actionssee 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)); | |||||
Done Inline ActionsYou 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… | |||||
Done Inline ActionsI 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)) { | |||||
Done Inline Actionsextra 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) { | |||||
Done Inline Actionsextra 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 */ | |||||
Not Done Inline ActionsTODO comment :) eadler: TODO comment :) | |||||
continue; | |||||
} | |||||
if (verbose > 2) | |||||
printf("File: %s\n", cf_key); | |||||
Done Inline ActionsThere'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 == false) { | |||||
Done Inline Actionsif (!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) { | |||||
Done Inline ActionsThe 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) {` | |||||
Done Inline ActionsNot 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 { | |||||
Done Inline ActionsYou 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);` | |||||
Done Inline Actionsok 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); | |||||
Done Inline ActionsUse 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… | |||||
Done Inline Actionsthis 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… | |||||
} | |||||
Done Inline Actionstarget->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; | |||||
Done Inline Actionstarget->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", | |||||
Done Inline Actionsextra parens, you don't need them around function calls cse_cem_gmail_com: extra parens, you don't need them around function calls | |||||
Done Inline Actionsthose 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; | |||||
Done Inline ActionsProbably 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); | |||||
} | |||||
Done Inline Actionsstyle(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) | |||||
Done Inline ActionsProbably: 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); | |||||
Done Inline ActionsAh, 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) | |||||
{ | |||||
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); | |||||
Not Done Inline ActionsThis 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); | |||||
} | |||||
Done Inline ActionsThese 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, "bzip", 4) == 0) { | |||||
Done Inline Actionsthese 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) { | |||||
target->compress = COMPRESS_XZ; | |||||
} else if (strcasecmp(val, "z") == 0 || strcasecmp(val, "gzip") == 0) { | |||||
Done Inline ActionsMaybe just strcasecmp() for "yes"? cse_cem_gmail_com: Maybe just `strcasecmp()` for "yes"? | |||||
target->compress = COMPRESS_GZIP; | |||||
} else if (strlen(val) == 0 || strcasecmp(val, "none") == 0) { | |||||
target->compress = COMPRESS_NONE; | |||||
} else { | |||||
warnx("Invalid flag '%s' in section '%s'\n", | |||||
val, ucl_object_key(top)); | |||||
return (false); | |||||
} | |||||
Done Inline ActionsNo 'n' needed for xz, I think? cse_cem_gmail_com: No 'n' needed for xz, I think? | |||||
Done Inline ActionsIt 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… | |||||
Done Inline ActionsAh, 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. | |||||
Done Inline ActionsThis question made me think about it more, and improve it further so gzip accepts: gz gzip (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 (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; | |||||
} | |||||
if (strcasecmp(val, "b") == 0 || strcasecmp(val, "binary") == 0) { | |||||
target->flags |= CE_BINARY; | |||||
Done Inline Actionsextra space cse_cem_gmail_com: extra space | |||||
} 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, "bzip", 4) == 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 || strcasecmp(val, "gzip") == 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 = NULL; | |||||
val = ucl_object_tostring_forced(obj); | |||||
ul = strtoul(val, &ep, 10); | |||||
if (ep == val) { | |||||
target->hours = 0; | |||||
Done Inline Actionsif ul == 0, check errno? eadler: if ul == 0, check errno? | |||||
} 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 { | |||||
target->hours = ul; | |||||
} | |||||
if (*ep == '\0' || strcmp(ep, "*") == 0) | |||||
return (true); | |||||
if (*ep != '@' && *ep != '$') | |||||
Done Inline Actionssame line cse_cem_gmail_com: same line | |||||
errx(1, "malformed interval '%s' in section '%s'\n", | |||||
val, ucl_object_key(top)); | |||||
Done Inline Actionsalso same line cse_cem_gmail_com: also same line | |||||
target->flags |= CE_TRIMAT; | |||||
target->trim_at = ptime_init(NULL); | |||||
ptm_opts = PTM_PARSE_ISO8601; | |||||
if (*ep == '$') { | |||||
Done Inline Actionssame cse_cem_gmail_com: same | |||||
ptm_opts = PTM_PARSE_DWM; | |||||
} | |||||
ptm_opts |= PTM_PARSE_MATCHDOM; | |||||
res = ptime_relparse(target->trim_at, ptm_opts, | |||||
ptimeget_secs(timenow), ep + 1); | |||||
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", | |||||
Done Inline Actionsbraces can be dropped here. cse_cem_gmail_com: braces can be dropped here. | |||||
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); | |||||
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); | |||||
Done Inline Actionssame cse_cem_gmail_com: same | |||||
} | |||||
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); | ||||
} | } | ||||
/* | /* | ||||
* 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. | ||||
Done Inline ActionsYou 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… | |||||
* | * | ||||
* Support function for qsort() in delete_oldest_timelog(). | * Support function for qsort() in delete_oldest_timelog(). | ||||
*/ | */ | ||||
static int | static int | ||||
oldlog_entry_compare(const void *a, const void *b) | oldlog_entry_compare(const void *a, const void *b) | ||||
{ | { | ||||
const struct oldlog_entry *ola = a, *olb = b; | const struct oldlog_entry *ola = a, *olb = b; | ||||
▲ Show 20 Lines • Show All 1,215 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) | |||||
Done Inline Actionsextra 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) | |||||
Done Inline ActionsIf 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); | |||||
Done Inline ActionsCan 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 :-). | |||||
Done Inline ActionsThis 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… | |||||
Done Inline ActionsOk. cse_cem_gmail_com: Ok. | |||||
} | |||||
break; | |||||
} | |||||
if (found && map[i].callback != NULL) { | |||||
Done Inline Actionsif (!found && ...) cse_cem_gmail_com: `if (!found && ...)` | |||||
cse_cem_gmail_comUnsubmitted Done Inline ActionsTypo — I think you want !found? cse_cem_gmail_com: Typo — I think you want `!found`? | |||||
allanjudeAuthorUnsubmitted Done Inline Actionsgood 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); | |||||
Done Inline Actions(ret) cse_cem_gmail_com: (ret) | |||||
} | } |
Don't need the backslash. typedef isn't part of the preprocessor.