On Fri, Nov 17, 2023 at 09:46:56AM +0100, Sebastien Marie wrote:
> Alexandr Nedvedicky <sas...@fastmail.net> writes:
>
> > Hello,
> >
> > diff below seems to make empty log message go way.
> > we have to check if sig_alrm fired here in pflogd:
> >
> > [...]
> >
> > I believe read at line 92 returns with EINTER, so we jump to
> > line to 75. If ALARM fires the condition at line 79 is true,
> > because pflogd's alarm handlers calls pcap_breakloop():
> >
> > 174 void
> > 175 sig_alrm(int sig)
> > 176 {
> > 177 pcap_breakloop(hpcap);
> > 178 gotsig_alrm = 1;
> > 179 }
> >
> >
> > this makes me thinking the one-liner below is the fix we want.
> >
> > regards
> > sashan
> >
> > --------8<---------------8<---------------8<------------------8<--------
> > diff --git a/sbin/pflogd/pflogd.c b/sbin/pflogd/pflogd.c
> > index 271e46326ee..42ca066b7e7 100644
> > --- a/sbin/pflogd/pflogd.c
> > +++ b/sbin/pflogd/pflogd.c
> > @@ -732,7 +732,8 @@ main(int argc, char **argv)
> > ret = -1;
> > break;
> > }
> > - logmsg(LOG_NOTICE, "%s", pcap_geterr(hpcap));
> > + if (gotsig_alrm == 0)
> > + logmsg(LOG_NOTICE, "%s", pcap_geterr(hpcap));
> > }
> >
> > if (gotsig_close)
>
> Your description makes sense, and it fixes the empty log message.
> ok semarie@
>
How about this instead. pcap_dispatch() returns -1 on error and -2 (aka
PCAP_ERROR_BREAK) on interrupt. On interrupt there is no need to print
anything (no matter the signal). pcap_geterr() will only print something
when pcap_dispatch() returns with -1 (PCAP_ERROR).
Not sure if we should use PCAP_ERROR or -1. The manpage only mentions -1
but the header file has the defines.
--
:wq Claudio
Index: pflogd.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
diff -u -p -r1.65 pflogd.c
--- pflogd.c 12 Nov 2023 15:18:04 -0000 1.65
+++ pflogd.c 17 Nov 2023 09:38:30 -0000
@@ -725,7 +725,7 @@ main(int argc, char **argv)
while (1) {
np = pcap_dispatch(hpcap, PCAP_NUM_PKTS,
phandler, (u_char *)dpcap);
- if (np < 0) {
+ if (np == -1) {
if (!if_exists(interface)) {
logmsg(LOG_NOTICE, "interface %s went away",
interface);
