[EMAIL PROTECTED] (Chin Fang) writes:
> I tested your version late this morning. It worked as far as I could
> tell. However, I felt that efficiency wise we could do better, so I
> combined both yours and mine and formed another patch. Please let me
> know whether it works for you.
With only a slight loss of portability, you could rewrite the logging
functions in assembly.
If you can demonstrate any noticable (even easily measurable) overhead
incurred by wrapping read() in a procedure call, I'd like to hear
about it. What's the reasoning behind your speculation (there's
locality and all that to consider)? This list is, after all, the land
of `Profile, don't speculate.' A companion slogan might be: `Don't
offer up dangerously misleading information in public.'
DJB has invested considerable effort in creating an I/O library that
provides a much better interface than stdio, and there's absolutely no
reason not to take advantage of it when extending his software.
The abysmal quality of most of the code floating around on the net is
largely due to programmers who feel that it's somehow `more efficient'
to avoid abstraction and modularity. The other common excuse is that
it's `easier,' but this cannot possibly be true when the necessary
abstractions have already been written and debugged.
> I also spotted the reason why in my original, unfinished, patch, the
> second strerr_warn1() stayed silent. It should. After pop3_quit(),
> the program die()s, and I placed the strerr_warn1 after
> commands(&ssin,pop3commands); in the main(). Thus it became a still
> born :(
That'll teach me to try to debug something based on a diff alone. :)
[ from a modified version of a function in the patch I posted ]
> + {
> + msgbytes_str[fmt_ulong(msgbytes_str,msgbytesread)];
> + pid_str[fmt_uint(pid_str,getpid())];
The `= 0's before the semicolons in the above two statements weren't
there for decorative purposes. You won't get bit in this case,
though, since C guarantees that global arrays are zero initialized.
For completeness' sake, I'm appending an enhanced version of my
original patch. It logs the the total network traffic in both
directions as well as the number of bytes slurped for RETR and TOP
commands. Along with a checkpassword that logs the user name and PID,
it gives you a reasonably good idea of how much bandwidth is being
used and who's using it.
As before, this is works-on-my-machine ware.
^L
--
Louis Theran
Nokia Research Center
(Not speaking for Nokia.)
*** qmail-pop3d.c.orig Sat May 20 19:54:14 2000
--- qmail-pop3d.c Sun May 21 21:06:37 2000
***************
*** 17,29 ****
#include "timeoutread.h"
#include "timeoutwrite.h"
! void die() { _exit(0); }
int saferead(fd,buf,len) int fd; char *buf; int len;
{
int r;
r = timeoutread(1200,fd,buf,len);
if (r <= 0) die();
return r;
}
--- 17,86 ----
#include "timeoutread.h"
#include "timeoutwrite.h"
! #include "strerr.h"
+ /* @@@ patched to log the number of bytes per connection.
+ *
+ * A simple patch to make qmail-pop3d emit log lines of the form:
+ *
+ * qmail-pop3d <pid>: <nbytes> bytes read --- bytes read from files in the
+Maildir
+ * qmail-pop3d <pid>: <nbytes> bytes in --- total bytes read from the network
+ * qmail-pop3d <pid>: <nbytes> bytes out --- total bytes written to the network
+ *
+ * to stderr, which is assumed to be pointed at multilog
+ *
+ * DON'T BLINDLY ASSUME THIS WORKS, it has undergone only cursory testing
+ *
+ * --Louis Theran, 2000-05-21
+ *
+ * Changes from the 05-20 version:
+ * -- Now logging all network traffic
+ * -- Moved the call to log_bytes() into die() so it always gets called
+ *
+ * Feel free to redistribute and use this.
+ */
+
+
+
+ /* @@@ keep track of bytes slurped for RETR/TOP, and total network traffic in both
+ directions */
+ static unsigned long msgbytes = 0;
+ static unsigned long netinbytes = 0;
+ static unsigned long netoutbytes = 0;
+
+ /* @@@ generating log messages */
+ #define log(msg) \
+ strerr_warn6("qmail-pop3d ",pid,": ",bytes," ",(msg),0)
+
+ static void log_bytes()
+ {
+ char bytes[FMT_ULONG];
+ char pid[FMT_ULONG];
+
+ pid[fmt_uint(pid,getpid())] = 0;
+
+ bytes[fmt_ulong(bytes,msgbytes)] = 0;
+ log("bytes read");
+
+ bytes[fmt_ulong(bytes,netinbytes)] = 0;
+ log("bytes in");
+
+ bytes[fmt_ulong(bytes,netoutbytes)] = 0;
+ log("bytes out");
+ }
+
+ #undef log
+
+ void die() { log_bytes(); _exit(0); }
+
+ /* @@@ saferead() and safewrite() are attached to the streams reading and writing
+ to the network */
int saferead(fd,buf,len) int fd; char *buf; int len;
{
int r;
r = timeoutread(1200,fd,buf,len);
if (r <= 0) die();
+ netinbytes += r;
return r;
}
***************
*** 32,40 ****
--- 89,108 ----
int r;
r = timeoutwrite(1200,fd,buf,len);
if (r <= 0) die();
+ netoutbytes += r;
return r;
}
+ /* @@@ we'll use counted_read() whenever we open a message in pop3_top() */
+ static int counted_read(fd,buf,len) int fd; char *buf; int len;
+ {
+ int rv;
+
+ rv = read(fd,buf,len);
+ if (rv > 0) msgbytes += rv;
+ return rv;
+ }
+
char ssoutbuf[1024];
substdio ssout = SUBSTDIO_FDBUF(safewrite,1,ssoutbuf,sizeof ssoutbuf);
***************
*** 268,274 ****
fd = open_read(m[i].fn);
if (fd == -1) { err_nosuch(); return; }
okay();
! substdio_fdbuf(&ssmsg,read,fd,ssmsgbuf,sizeof(ssmsgbuf));
blast(&ssmsg,limit);
close(fd);
}
--- 336,342 ----
fd = open_read(m[i].fn);
if (fd == -1) { err_nosuch(); return; }
okay();
! substdio_fdbuf(&ssmsg,counted_read,fd,ssmsgbuf,sizeof(ssmsgbuf));
blast(&ssmsg,limit);
close(fd);
}
