On Mon, Jul 15, 2002 at 04:21:04PM -0700, Fulvio Risso wrote:
> 1. widespread use of u_int variables.
> There is no assurance about the size of a u_int variable. In fact, u_int
> is usually defined by the operating system as a 'unsigned int', which
> can be either 32 or 64 bits.
Most OSes and compilers are probably either ILP32 or LP64, so an
"unsigned int" will probably be 32 bits.
However, as you note there's no guarantee of that.
> Vice versa, 'bpf_u_int32' should be defined
> in order to be a 32bit number.
> Should we change the 'u_int' to 'bpf_u_int32' in pcap.h (this could be
> done, for instance, into the 'struct pcap_stat', 'struct pcap_if')?
That's probably not a bad idea, although I suspect it's not necessary in
practice. ("u_int" isn't used in any data structures that go into
files, for example, so, as long as it has the same size on a given
platform, that's probably enough.)
Note, though, that we also have "u_short"s, which are likely to be 16
bits, but aren't *guaranteed* to be 32 bits.
> 2. use of structure 'sockaddr' in 'struct pcap_addr'
> This type does not support IPv6 addresses, since it is only 14 bytes
> long.
> Should we use the new 'sockaddr_storage' instead?
I'm not sure it's present on all platforms.
If we use that, we should probably define "sockaddr_storage" to be just
"sockaddr" on platforms where there's no "struct sockaddr_storage";
either:
those platforms have a newer BSD-style "struct sockaddr", which
is really used just as a template (you cast the "struct
sockaddr" pointer to a pointer to the appropriate type of
address), and which contains a length field at the beginning, so
"dup_sockaddr()" will copy all of the *actual* socket address
or
those platforms don't have a newer BSD-style "struct sockaddr"
in which case they can't handle IPv6 anyway.
(There *are* platforms, such as Solaris 8, that do have IPv6 support,
that have "struct sockaddr_storage", and that don't have a newer
BSD-style "struct sockaddr", so there are platforms where "struct
sockaddr" isn't good enough.)
The problem there is that we'd have to put the #definition of
"sockaddr_storage" - or the #definition of HAVE_SOCKADDR_STORAGE or
something such as that - *into "pcap.h" itself*. A configure script
that stuffs it into "config.h" would be fine when compiling libpcap
itself; however, it doesn't help for programs that *use* libpcap, as
they'd have to do that in their *own* configure scripts.
(For a real-world example of this kind of mess, caused by some library
having a configure script that generates a "config.h" *AND* having
*exported* header files whose correct behavior, in programs using that
librar, requires that stuff be #defined the same way that the platform's
internal "config.h" script defines them, take a look at the big comment
in "wiretap/file_wrappers.c" in Ethereal:
/*
* OK, now this is tricky.
*
* At least on FreeBSD 3.2, "/usr/include/zlib.h" includes
* "/usr/include/zconf.h", which, if HAVE_UNISTD_H is defined,
* #defines "z_off_t" to be "off_t", and if HAVE_UNISTD_H is
* not defines, #defines "z_off_t" to be "long" if it's not
* already #defined.
*
* In 4.4-Lite-derived systems such as FreeBSD, "off_t" is
* "long long int", not "long int", so the definition of "z_off_t" -
* and therefore the types of the arguments to routines such as
* "gzseek()", as declared, with prototypes, in "zlib.h" - depends
* on whether HAVE_UNISTD_H is defined prior to including "zlib.h"!
*
* It's not defined in the FreeBSD 3.2 "zlib", so if we include "zlib.h"
* after defining HAVE_UNISTD_H, we get a misdeclaration of "gzseek()",
* and, if we're building with "zlib" support, anything that seeks
* on a file may not work.
*
* Other BSDs may have the same problem, if they haven't done something
* such as defining HAVE_UNISTD_H in "zconf.h".
*
* "config.h" defines HAVE_UNISTD_H, on all systems that have it, and all
* 4.4-Lite-derived BSDs have it. Therefore, given that "zlib.h" is included
* by "file_wrappers.h", that means that unless we include "zlib.h" before
* we include "config.h", we get a misdeclaration of "gzseek()".
*
* Unfortunately, it's "config.h" that tells us whether we have "zlib"
* in the first place, so we don't know whether to include "zlib.h"
* until we include "config.h"....
*
* A similar problem appears to occur with "gztell()", at least on
* NetBSD.
*
* To add further complication, on recent versions, at least, of OpenBSD,
* the Makefile for zlib defines HAVE_UNISTD_H.
*
* So what we do is, on all OSes other than OpenBSD, *undefine* HAVE_UNISTD_H
* before including "wtap.h" (we need "wtap.h" to get the WTAP_ERR_ZLIB
* values, and it also includes "zlib.h" if HAVE_ZLIB" is defined), and,
* if we have zlib, make "file_seek()" and "file_tell()" subroutines, so
* that the only calls to "gzseek()" and "gztell()" are in this file, which,
* by dint of the hackery described above, manages to correctly declare
* "gzseek()" and "gztell()".
*
* On OpenBSD, we forcibly *define* HAVE_UNISTD_H if it's not defined.
*
* Hopefully, the BSDs will, over time, remove the test for HAVE_UNISTD_H
* from "zconf.h", so that "gzseek()" and "gztell()" will be declared
* with the correct signature regardless of whether HAVE_UNISTD_H is
* defined, so that if they change the signature we don't have to worry
* about making sure it's defined or not defined.
*
* DO NOT, UNDER ANY CIRCUMSTANCES, REMOVE THE FOLLOWING LINES, OR MOVE
* THEM AFTER THE INCLUDE OF "wtap.h"! Doing so will cause any program
* using Wiretap to read capture files to fail miserably on a FreeBSD
* 3.2 or 3.3 system - and possibly some other BSD systems - if zlib is
* installed. If you *must* include <unistd.h> here, do so *before*
* including "wtap.h", and before undefining HAVE_UNISTD_H. If you
* *must* have HAVE_UNISTD_H defined before including "wtap.h", put
* "file_error()" into a file by itself, which can cheerfully include
* "wtap.h" and get "gzseek()" misdeclared, and include just "zlib.h"
* in this file - *after* undefining HAVE_UNISTD_H.
*/
Ugly.)
-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
