On Mar 19, 2009, at 2:39 AM, Shaked, Nitzan wrote:
2) I believe the current code doesn't have the notion of "non-
blocking",
which it doesn't have the notion of "I read only part of what was
request, maybe half a packet, or half a header",
...and it would even need that if you got rid of buffering in the
savefile code, as, when select() says the descriptor is readable,
there's no guarantee that what will be readable from the pipe will be
a collection of one or more full records with no fraction of a record
at the end.
1) Encapsulation. From your suggestion to put the fd in non-blocking
mode, I understand you mean to actually call fcntl() myself (not from
inside libpcap, since pcap_setnonblock() does nothing on savefiles).
This means a flow which goes something like this:
if ( is_tty(filename) || is_pipe(filename) ||
is_socket(filename) ... )
{
fp = fopen( ... );
fcntl( fp, ... );
(fcntl() doesn't work on FILE *'s, it works on file descriptors, so
you'd have to call fileno().)
pcap_fopen_offline( fp, ... );
}
... I can't use pcap_open_offline(), since it will do its own fopen()
and I'll lose the non-blocking effect.
No, you'd call pcap_file() and then call fileno() and call fcntl() on
that.
Or, given that, as indicated above, in order to use select() when
reading from a pipe we'd have to change the savefile code *anyway*, we
could make pcap_setnonblock() to work on savefiles.
So in effect pcap_open_offline()
becomes redundant. Everybody will have to know about the special
non-blocking thing, do the logic I just mentioned above, and call
pcap_fopen_offline().
Everybody *who needs to use select()* would do so. *NOT* all
applications using pcap_open_offline() need to use select(); tcpdump,
for example, doesn't.
2) Bug (?) in the current code. In savefile.c, in
pcap_fopen_offline(),
there's an fread() to read the savefile's header. The fread goes
something like "amt_read = fread( &hdr, 1, sizeof(hfr), fp )", and
then
asking "if ( amt_read != sizeof(hdr) )". So we're asking to read
sizeof(hdr) chars (portability issue: not all platforms have 8bit
chars).
(I'll wait until somebody has a version of libpcap() running on
TOPS-10 or TOPS-20 or Unix-on-Univac-mainframes or... before we worry
about that. Programs using libpcap/WinPcap would have plenty of
portability issues on those platforms over and above any portability
issues libpcap has; all the current platforms on which libpcap runs
have 8-bit bytes.)
If we're in non-blocking mode, and reading from a pipe, say, it
could be that there are only 4 bytes available right now, but there
will
be more available later.
If we're in blocking mode, the same could be true, as indicated
above. If you really want to be able to use libpcap to read from a
pipe, without blocking indefinitely on the pipe, with the aid of
select(), the code *has* to be able to deal with that, even if it does
only one-byte read() calls, as select() might tell you that there's
only one byte to read.
4) Regarding the SF_ERR_AGAIN I mention above, and regarding the
translation of return values from sf_next_packet() and
pcap_offline_read() I mention: once we introduce the notion of
non-blocking IO, it seems to me that we need to introduce new return
values all across:
4.1) sf_next_packet() should return:
It can do whatever's necessary, as it's static.
4.2) pcap_offline_read() should return:
...
OFFLINE_READ_BREAK_LOOP (value: -3), if we were told to break
out of the loop
No, that breaks binary compatibility. It'd have to continue to return
-2 when told to break out of the loop.
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.