From: Baptiste Daroussin Date: Thu, 5 Jan 2023 13:35:50 +0000 (+0100) Subject: log_oper: rework X-Git-Tag: RELEASE_1_4_0_a2~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=299c4e2154b26c1a906f69e994dfc089cb36f7de;p=thirdparty%2Fmlmmj.git log_oper: rework use file descriptor and do not use an intermediary variable before writting the logs, as a side effect it drop the limitation of 256 chars per lines --- diff --git a/ChangeLog b/ChangeLog index 7b05bae8..d83016c2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,7 @@ o mlmmj-send does not need anymore absolute path o Use copy_file_range if available o Use arc4random_uniform if available + o Logs are not limited anymore to 256 characters per lines 1.4.0-a1 o Add a test suite o Modernize code (dprintf, posix_spawn, asprintf, getline, daemon, ...) diff --git a/include/log_oper.h b/include/log_oper.h index a57d3c39..78902c99 100644 --- a/include/log_oper.h +++ b/include/log_oper.h @@ -1,6 +1,6 @@ -/* Copyright (C) 2005 Mads Martin Joergensen - * - * $Id$ +/* + * Copyright (C) 2005 Mads Martin Joergensen + * Copyright (C) 2023 Baptiste Daroussin * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to @@ -24,6 +24,6 @@ #ifndef LOG_OPER_H #define LOG_OPER_H -int log_oper(const char *prefix, const char *basename, const char *fmt, ...); +int log_oper(int listfd, const char *basename, const char *fmt, ...); #endif /* LOG_OPER_H */ diff --git a/src/listcontrol.c b/src/listcontrol.c index 753bb25e..eb7c7ce6 100644 --- a/src/listcontrol.c +++ b/src/listcontrol.c @@ -238,9 +238,8 @@ int listcontrol(strlist *fromemails, const char *listdir, tll_front(*fromemails), mlmmjsend, ctrlfd); return -1; } - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: request for digest" - " subscription from %s", - tll_front(*fromemails)); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: request for digest" + " subscription from %s", tll_front(*fromemails)); exec_or_die(mlmmjsub, "-L", listdir, "-a", tll_front(*fromemails), "-d", "-r", "-c", (nosubconfirm ? NULL : "-C"), NULL); @@ -280,9 +279,8 @@ int listcontrol(strlist *fromemails, const char *listdir, tll_front(*fromemails), mlmmjsend, ctrlfd); return -1; } - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: request for nomail" - " subscription from %s", - tll_front(*fromemails)); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: request for nomail" + " subscription from %s", tll_front(*fromemails)); exec_or_die(mlmmjsub, "-L", listdir, "-a", tll_front(*fromemails), "-n", "-r", "-c", (nosubconfirm ? NULL : "-C"), NULL); @@ -322,9 +320,8 @@ int listcontrol(strlist *fromemails, const char *listdir, tll_front(*fromemails), mlmmjsend, ctrlfd); return -1; } - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: request for both" - " subscription from %s", - tll_front(*fromemails)); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: request for both" + " subscription from %s", tll_front(*fromemails)); exec_or_die(mlmmjsub, "-L", listdir, "-a", tll_front(*fromemails), "-b", "-r", "-c", (nosubconfirm ? NULL : "-C"), NULL); @@ -346,9 +343,8 @@ int listcontrol(strlist *fromemails, const char *listdir, " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: request for regular" - " subscription from %s", - tll_front(*fromemails)); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: request for regular" + " subscription from %s", tll_front(*fromemails)); exec_or_die(mlmmjsub, "-L", listdir, "-a", tll_front(*fromemails), "-r", "-c", (nosubconfirm ? NULL : "-C"), NULL); @@ -385,8 +381,8 @@ int listcontrol(strlist *fromemails, const char *listdir, tmpstr = mygetline(tmpfd); chomp(tmpstr); close(tmpfd); - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: %s confirmed" - " subscription to %s", tmpstr, subtypename); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: %s confirmed" + " subscription to %s", tmpstr, subtypename); exec_or_die(mlmmjsub, "-L", listdir, "-a", tmpstr, "-n", "-R", "-c", NULL); break; @@ -407,9 +403,8 @@ int listcontrol(strlist *fromemails, const char *listdir, chomp(tmpstr); close(tmpfd); unlink(conffilename); - log_oper(listdir, OPLOGFNAME, "mlmmj-sub: %s confirmed" - " subscription to regular list", - tmpstr); + log_oper(listfd, OPLOGFNAME, "mlmmj-sub: %s confirmed" + " subscription to regular list", tmpstr); exec_or_die(mlmmjsub, "-L", listdir, "-a", tmpstr, "-R", "-c", NULL); break; @@ -434,9 +429,8 @@ int listcontrol(strlist *fromemails, const char *listdir, " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "mlmmj-unsub: %s requests" - " unsubscribe", - tll_front(*fromemails)); + log_oper(listfd, OPLOGFNAME, "mlmmj-unsub: %s requests" + " unsubscribe", tll_front(*fromemails)); exec_or_die(mlmmjunsub, "-L", listdir, "-a", tll_front(*fromemails), "-r", (nosubconfirm ? "-c" : "-C"), NULL); @@ -464,8 +458,8 @@ int listcontrol(strlist *fromemails, const char *listdir, close(tmpfd); chomp(tmpstr); unlink(conffilename); - log_oper(listdir, OPLOGFNAME, "mlmmj-unsub: %s confirmed" - " unsubscribe from digest", tmpstr); + log_oper(listfd, OPLOGFNAME, "mlmmj-unsub: %s confirmed" + " unsubscribe from digest", tmpstr); exec_or_die(mlmmjunsub, "-L", listdir, "-a", tmpstr, "-d", "-R", "-c", NULL); break; @@ -492,8 +486,8 @@ int listcontrol(strlist *fromemails, const char *listdir, close(tmpfd); chomp(tmpstr); unlink(conffilename); - log_oper(listdir, OPLOGFNAME, "mlmmj-unsub: %s confirmed" - " unsubscribe from nomail", tmpstr); + log_oper(listfd, OPLOGFNAME, "mlmmj-unsub: %s confirmed" + " unsubscribe from nomail", tmpstr); exec_or_die(mlmmjunsub, "-L", listdir, "-a", tmpstr, "-n", "-R", "-c", NULL); break; @@ -520,9 +514,8 @@ int listcontrol(strlist *fromemails, const char *listdir, close(tmpfd); chomp(tmpstr); unlink(conffilename); - log_oper(listdir, OPLOGFNAME, "mlmmj-unsub: %s confirmed" - " unsubscribe from regular list", - tmpstr); + log_oper(listfd, OPLOGFNAME, "mlmmj-unsub: %s confirmed" + " unsubscribe from regular list", tmpstr); exec_or_die(mlmmjunsub, "-L", listdir, "-a", tmpstr, "-R", "-c", NULL); break; @@ -601,7 +594,7 @@ int listcontrol(strlist *fromemails, const char *listdir, free(moderatefilename); - log_oper(listdir, OPLOGFNAME, "%s released %s", + log_oper(listfd, OPLOGFNAME, "%s released %s", tll_front(*fromemails), param); if (omit != NULL) @@ -622,7 +615,7 @@ int listcontrol(strlist *fromemails, const char *listdir, " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "%s rejected %s", + log_oper(listfd, OPLOGFNAME, "%s rejected %s", tll_front(*fromemails), param); free(param); if (unlink(moderatefilename) != 0) { @@ -648,7 +641,7 @@ permit: " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "%s permitted %s", + log_oper(listfd, OPLOGFNAME, "%s permitted %s", tll_front(*fromemails), param); exec_or_die(mlmmjsub, "-L", listdir, "-m", param, "-c", NULL); break; @@ -666,7 +659,7 @@ permit: " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "%s obstructed %s", + log_oper(listfd, OPLOGFNAME, "%s obstructed %s", tll_front(*fromemails), param); free(param); if (unlink(gatekeepfilename) != 0) { @@ -688,7 +681,7 @@ permit: " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "%s requested help", + log_oper(listfd, OPLOGFNAME, "%s requested help", tll_front(*fromemails)); txt = open_text(listdir, "help", NULL, NULL, NULL, "listhelp"); MY_ASSERT(txt); @@ -710,7 +703,7 @@ permit: " Ignoring mail"); return -1; } - log_oper(listdir, OPLOGFNAME, "%s requested faq", + log_oper(listfd, OPLOGFNAME, "%s requested faq", tll_front(*fromemails)); txt = open_text(listdir, "faq", NULL, NULL, NULL, "listfaq"); MY_ASSERT(txt); @@ -758,7 +751,7 @@ permit: log_error(LOG_ARGS, "Unable to open archive file"); exit(EXIT_FAILURE); } - log_oper(listdir, OPLOGFNAME, "%s got archive/%s", + log_oper(listfd, OPLOGFNAME, "%s got archive/%s", tll_front(*fromemails), archivefilename); exec_or_die(mlmmjsend, "-T", tll_front(*fromemails), "-L", listdir, "-l", "6", "-m", archivefilename, "-a", "-D", @@ -775,7 +768,7 @@ permit: tll_foreach(*owners, it) { if(strcasecmp(tll_front(*fromemails), it->item) == 0) { - log_oper(listdir, OPLOGFNAME, + log_oper(listfd, OPLOGFNAME, "%s requested sub list", tll_front(*fromemails)); owner = it->item; diff --git a/src/log_oper.c b/src/log_oper.c index 82ab11dc..b99e04b0 100644 --- a/src/log_oper.c +++ b/src/log_oper.c @@ -32,68 +32,55 @@ #include #include +#include "xmalloc.h" #include "mlmmj.h" #include "log_error.h" #include "log_oper.h" -#include "strgen.h" #include "wrappers.h" #include "utils.h" -int log_oper(const char *prefix, const char *basename, const char *fmt, ...) +int log_oper(int listfd, const char *basename, const char *fmt, ...) { int fd, statres; - char ct[26], *logfilename, *tmp, log_msg[256]; + char ct[26], *tmp; struct stat st; time_t t; va_list ap; - size_t i; - logfilename = concatstr(3, prefix, "/", basename); - statres = lstat(logfilename, &st); + statres = fstatat(listfd, basename, &st, AT_SYMLINK_NOFOLLOW); if(statres < 0 && errno != ENOENT) { - log_error(LOG_ARGS, "Could not stat logfile %s", logfilename); - free(logfilename); + log_error(LOG_ARGS, "Could not stat logfile %s", basename); return -1; } - + if(statres >= 0 && st.st_size > (off_t)OPLOGSIZE) { - tmp = concatstr(2, logfilename, ".rotated"); - if(rename(logfilename, tmp) < 0) { + xasprintf(&tmp, "%s.rotated", basename); + if(renameat(listfd, basename, listfd, tmp) < 0) { log_error(LOG_ARGS, "Could not rename %s,%s", - logfilename, tmp); + basename, tmp); } free(tmp); } - - fd = open(logfilename, O_RDWR|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR); + + fd = openat(listfd, basename, O_RDWR|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR); if(fd < 0 && !lock(fd, true)) { - log_error(LOG_ARGS, "Could not open %s", logfilename); - free(logfilename); + log_error(LOG_ARGS, "Could not open %s", basename); return -1; } if((time(&t) == (time_t)-1) || (ctime_r(&t, ct) == NULL)) strncpy(ct, "Unknown time", sizeof(ct)); - else - ct[24] = '\0'; va_start(ap, fmt); - i = vsnprintf(log_msg, sizeof(log_msg), fmt, ap); - if(i < 0) { - va_end(ap); - log_error(LOG_ARGS, "Failed to format log message: %s", fmt); - return -1; - } - if(i > sizeof(log_msg)) - log_error(LOG_ARGS, "Log message truncated"); - + if (dprintf(fd, "%s", ct) < 0) + log_error(LOG_ARGS, "Could not write to %s", basename); + if (vdprintf(fd, fmt, ap) <0) + log_error(LOG_ARGS, "Could not write to %s", basename); + if (dprintf(fd, "\n") < 0) + log_error(LOG_ARGS, "Could not write to %s", basename); va_end(ap); - if(dprintf(fd, "%s %s\n", ct, log_msg) < 0) - log_error(LOG_ARGS, "Could not write to %s", logfilename); - close(fd); - free(logfilename); - + return 0; } diff --git a/src/mlmmj-process.c b/src/mlmmj-process.c index 6b1e2bd1..a4f4f8fa 100644 --- a/src/mlmmj-process.c +++ b/src/mlmmj-process.c @@ -223,7 +223,7 @@ static void newmoderated(const char *listdir, const char *mailfilename, static enum action do_access(strlist *rule_strs, strlist *hdrs, - const char *from, const char *listdir) + const char *from, int listfd) { unsigned int match; char *rule_ptr; @@ -257,7 +257,7 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, log_error(LOG_ARGS, "Unable to parse rule #%d \"%s\":" " Missing action keyword. Denying post from \"%s\"", i, it->item, from); - log_oper(listdir, OPLOGFNAME, "Unable to parse rule #%d \"%s\":" + log_oper(listfd, OPLOGFNAME, "Unable to parse rule #%d \"%s\":" " Missing action keyword. Denying post from \"%s\"", i, it->item, from); return DENY; @@ -267,7 +267,7 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, rule_ptr++; } else if (*rule_ptr == '\0') { /* the rule is a keyword and no regexp */ - log_oper(listdir, OPLOGFNAME, "mlmmj-process: access -" + log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -" " A mail from \"%s\" was %s by rule #%d \"%s\"", from, action_strs[act], i, it->item); return act; @@ -277,7 +277,7 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, log_error(LOG_ARGS, "Unable to parse rule #%d \"%s\":" " Invalid character after action keyword." " Denying post from \"%s\"", i, it->item, from); - log_oper(listdir, OPLOGFNAME, "Unable to parse rule #%d \"%s\":" + log_oper(listfd, OPLOGFNAME, "Unable to parse rule #%d \"%s\":" " Invalid character after action keyword." " Denying post from \"%s\"", i, it->item, from); return DENY; @@ -309,7 +309,7 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, " (message: '%s') (expression: '%s')" " Denying post from \"%s\"", i, it->item, errbuf, rule_ptr, from); - log_oper(listdir, OPLOGFNAME, "regcomp() failed for rule" + log_oper(listfd, OPLOGFNAME, "regcomp() failed for rule" " #%d \"%s\" (message: '%s') (expression: '%s')" " Denying post from \"%s\"", i, it->item, errbuf, rule_ptr, from); @@ -330,12 +330,12 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, if (match != not) { if (match) { - log_oper(listdir, OPLOGFNAME, "mlmmj-process: access -" + log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -" " A mail from \"%s\" with header \"%s\" was %s by" " rule #%d \"%s\"", from, hdr, action_strs[act], i, it->item); } else { - log_oper(listdir, OPLOGFNAME, "mlmmj-process: access -" + log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -" " A mail from \"%s\" was %s by rule #%d \"%s\"" " because no header matched.", from, action_strs[act], i, it->item); @@ -345,7 +345,7 @@ static enum action do_access(strlist *rule_strs, strlist *hdrs, i++; } - log_oper(listdir, OPLOGFNAME, "mlmmj-process: access -" + log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -" " A mail from \"%s\" didn't match any rules, and" " was denied by default.", from); return DENY; @@ -757,7 +757,7 @@ int main(int argc, char **argv) close(rawmailfd); close(donemailfd); unlink(mailfile); - log_oper(listdir, OPLOGFNAME, "mlmmj-process: sending" + log_oper(listfd, OPLOGFNAME, "mlmmj-process: sending" " mail from %s to owner", efrom); exec_or_die(mlmmjsend, "-l", "4", "-L", listdir, "-F", @@ -900,7 +900,7 @@ int main(int argc, char **argv) /* Don't send a mail about denial to the list, but silently * discard and exit. Also do this in case it's turned off */ accret = do_access(access_rules, &allheaders, - posteraddr, listdir); + posteraddr, listfd); if (accret == DENY) { if ((strcasecmp(listaddr, posteraddr) == 0) || statctrl(ctrlfd, "noaccessdenymails")) {