tcpdump mailing list archives
Re: pcap_offline_read return value
From: Guy Harris <guy () alum mit edu>
Date: Mon, 12 Dec 2016 16:34:32 -0800
On Dec 12, 2016, at 6:53 AM, Martin Dubuc <martind1111 () gmail com> wrote:
I was testing some of the libpcap API and would like to get some clarification on the return value of one of the function. Is pcap_dispatch supposed to return the number of packets read/processed?
If it successfully processes any packets, i.e. hands them to the callback, it should return the count of packets passed to the callback. Not all packets read will necessarily be handed to the callback; if there's a filter on the pcap_t, packets that don't pass the filter aren't handed to the callback and aren't counted. If there are no packets left, it should return 0. If an error occurred, it should return an error value (negative number). If pcap_breakloop() was called by the callback or from another thread or from a signal handler, it should return -2.
In my sample code, it always return 0. I tried to trace the code execution and found out that pcap_offline_read returned 0 instead of packet count if the return value comes from executing the following block: status = p->next_packet_op(p, &h, &data); if (status) { if (status == 1) return (0); return (status); } p->next_packet_op translates to pcap_next_packet and this function returns 0 on success or 1 if there are no more packets. In both cases, 0 will be returned.
If p->next_packet_op returns 1, which means "no more packets", pcap_offline_read() will immediately return 0. What it *should* do is break out of the loop, and return the number of packets that have already been processed.
This is the case in both 1.7.4 and 1.8.1.
It's also the case, albeit with a different code path, in 0.4 (the last release from LBL): /* * Read sf_readfile and return the next packet. Return the header in hdr * and the contents in buf. Return 0 on success, SFERR_EOF if there were * no more packets, and SFERR_TRUNC if a partial packet was encountered. */ static int sf_next_packet(pcap_t *p, struct pcap_pkthdr *hdr, u_char *buf, int buflen) { FILE *fp = p->sf.rfile; /* read the stamp */ if (fread((char *)hdr, sizeof(struct pcap_pkthdr), 1, fp) != 1) { /* probably an EOF, though could be a truncated packet */ return (1); } ... if (hdr->caplen > buflen) { ... if (fread((char *)tp, hdr->caplen, 1, fp) != 1) { sprintf(p->errbuf, "truncated dump file"); return (-1); } /* * We can only keep up to buflen bytes. Since caplen > buflen * is exactly how we got here, we know we can only keep the * first buflen bytes and must drop the remainder. Adjust * caplen accordingly, so we don't get confused later as * to how many bytes we have to play with. */ hdr->caplen = buflen; memcpy((char *)buf, (char *)tp, buflen); } else { /* read the packet itself */ if (fread((char *)buf, hdr->caplen, 1, fp) != 1) { sprintf(p->errbuf, "truncated dump file"); return (-1); } } return (0); } ... /* * Print out packets stored in the file initialized by sf_read_init(). * If cnt > 0, return after 'cnt' packets, otherwise continue until eof. */ int pcap_offline_read(pcap_t *p, int cnt, pcap_handler callback, u_char *user) { struct bpf_insn *fcode = p->fcode.bf_insns; int status = 0; int n = 0; while (status == 0) { struct pcap_pkthdr h; status = sf_next_packet(p, &h, p->buffer, p->bufsize); if (status) { if (status == 1) return (0); return (status); } if (fcode == NULL || bpf_filter(fcode, p->buffer, h.len, h.caplen)) { (*callback)(user, &h, p->buffer); if (++n >= cnt && cnt > 0) break; } } /*XXX this breaks semantics tcpslice expects */ return (n); }
Is this intended?
Probably not, but it's a long-standing bug. *Hopefully*, no software depends on it returning 0 if the loop wasn't terminated by exhausting the count passed to pcap_dispatch(). _______________________________________________ tcpdump-workers mailing list tcpdump-workers () lists tcpdump org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Current thread:
- pcap_offline_read return value Martin Dubuc (Dec 12)
- Re: pcap_offline_read return value Guy Harris (Dec 12)