Replace exit() with the new exit_tcpdump(), which calls smiExit()

[bijalthanawala]

As suggested by @guyharris here - #529:

New function exit_tcpdump(), which calls smiExit()(if USE_LIBSMI is defined), introduced.
All calls to exit(), including the one in utils.c:error() replaced with call to this new function.

smiExit() is leaking one memory allocation, which libsmi team has been informed of along
with some preliminary investigation report and offer to assist.

Before calling smiExit():

$ valgrind tcpdump -h
.
.
.
==4311==
==4311== HEAP SUMMARY:
==4311== in use at exit: 320,870 bytes in 3,830 blocks
==4311== total heap usage: 6,445 allocs, 2,615 frees, 586,344 bytes allocated
==4311==
==4311== LEAK SUMMARY:
==4311== definitely lost: 744 bytes in 53 blocks
==4311== indirectly lost: 0 bytes in 0 blocks
==4311== possibly lost: 0 bytes in 0 blocks
==4311== still reachable: 320,126 bytes in 3,777 blocks
==4311== suppressed: 0 bytes in 0 blocks
==4311== Rerun with --leak-check=full to see details of leaked memory
.
.
.

After calling smiExit():

$ valgrind tcpdump -h
.
.
.
==4320==
==4320== HEAP SUMMARY:
==4320== in use at exit: 816 bytes in 54 blocks
==4320== total heap usage: 6,445 allocs, 6,391 frees, 586,344 bytes allocated
==4320==
==4320== LEAK SUMMARY:
==4320== definitely lost: 744 bytes in 53 blocks
==4320== indirectly lost: 0 bytes in 0 blocks
==4320== possibly lost: 0 bytes in 0 blocks
==4320== still reachable: 72 bytes in 1 blocks
==4320== suppressed: 0 bytes in 0 blocks
==4320== Rerun with --leak-check=full to see details of leaked memory
.
.
.

[guyharris]

The smi_config man page doesn't say what happens if you call smiExit() without having called smiInit(), so you might want to have a static smiInit_called flag in tcpdump.c, set it after calling smiInit() if smiInit() returns 0 (meaning "success"), and only call smiExit() if it's non-zero.

[guyharris]

Please use tabs - or 8 spaces - for indentation, for consistency with the rest of the code.

[infrastation]

It seems to me the code in print.c and print-esp.c belongs to libnetdissect and as such should not call back into tcpdump. Or libsmi coupling (i.e. both initialization and teardown) should belong to libnetdissect.

[bijalthanawala]

@infrastation : Thanks for alerting.
I looked at the Makefile (as generated by ./configure on my system) and in order to understand this I tried building just the 'tcpdump' - with an error intentionally inserted (misspelled exit_tcpdum) in print.c - and it seems from the output below that print.o and print-esp.o (highlighted) are statically linked to tcpdump.

Without past-knowledge or complete overview of the project, it is quite possible I am missing something.

$ make tcpdump
gcc -ffloat-store -DHAVE_CONFIG_H -D_U_=__ attribute__((unused)) -I. -g -O2 -o tcpdump setsignal.o tcpdump.o util.o version.o addrtoname.o addrtostr.o af.o ascii_strcasecmp.o checksum.o cpack.o gmpls.o gmt2local.o in_cksum.o ipproto.o l2vpn.o machdep.o nlpid.o oui.o parsenfsfh.o print.o print-802_11.o print-802_15_4.o print-ah.o print-ahcp.o print-aodv.o print-aoe.o print-ap1394.o print-arcnet.o print-arp.o print-ascii.o print-atalk.o print-atm.o print-babel.o print-beep.o print-bfd.o print-bgp.o print-bootp.o print-bt.o print-calm-fast.o print-carp.o print-cdp.o print-cfm.o print-chdlc.o print-cip.o print-cnfp.o print-dccp.o print-decnet.o print-dhcp6.o print-domain.o print-dtp.o print-dvmrp.o print-eap.o print-egp.o print-eigrp.o print-enc.o print-esp.o print-ether.o print-fddi.o print-forces.o print-fr.o print-frag6.o print-ftp.o print-geneve.o print-geonet.o print-gre.o print-hsrp.o print-http.o print-icmp.o print-icmp6.o print-igmp.o print-igrp.o print-ip.o print-ip6.o print-ip6opts.o print-ipcomp.o print-ipfc.o print-ipnet.o print-ipx.o print-isakmp.o print-isoclns.o print-juniper.o print-krb.o print-l2tp.o print-lane.o print-ldp.o print-lisp.o print-llc.o print-lldp.o print-lmp.o print-loopback.o print-lspping.o print-lwapp.o print-lwres.o print-m3ua.o print-medsa.o print-mobile.o print-mobility.o print-mpcp.o print-mpls.o print-mptcp.o print-msdp.o print-msnlb.o print-nflog.o print-nfs.o print-nsh.o print-ntp.o print-null.o print-olsr.o print-openflow-1.0.o print-openflow.o print-ospf.o print-ospf6.o print-otv.o print-pgm.o print-pim.o print-pktap.o print-ppi.o print-ppp.o print-pppoe.o print-pptp.o print-radius.o print-raw.o print-resp.o print-rip.o print-ripng.o print-rpki-rtr.o print-rrcp.o print-rsvp.o print-rt6.o print-rtsp.o print-rx.o print-sctp.o print-sflow.o print-sip.o print-sl.o print-sll.o print-slow.o print-smtp.o print-snmp.o print-stp.o print-sunatm.o print-sunrpc.o print-symantec.o print-syslog.o print-tcp.o print-telnet.o print-tftp.o print-timed.o print-tipc.o print-token.o print-udld.o print-udp.o print-usb.o print-vjc.o print-vqp.o print-vrrp.o print-vtp.o print-vxlan.o print-vxlan-gpe.o print-wb.o print-zephyr.o print-zeromq.o signature.o strtoaddr.o util-print.o print-smb.o smbutil.o strlcat.o strlcpy.o -lcrypto -lpcap -lsmi
print.o: In function 'ndo_error':
/home/bijal/projects/tcpdump/./print.c:433: undefined reference to `exit_tcpdum'
collect2: error: ld returned 1 exit status
Makefile:366: recipe for target 'tcpdump' failed
make: *** [tcpdump] Error 1

[infrastation]

You are right, tcpdump currently compiles all the source files in as that has been the traditional way. As far as I understand, it was intended to switch tcpdump to link with libnetdissect.a, which is compiled next after tcpdump from mostly the same files. This has not happened but when it happens there is going to be a problem because not every binary trying to link with libnetdissect will have exit_tcpdump().

Other developers may have a better understanding of this.

[guyharris]

OK, I've done it differently in 91e08f8. libnetdissect now has nd_init() and nd_cleanup() routines, the first of which does whatever initialization is needed for libnetdissect, such as initializing Winsock and libsmi, and the second of which cleans up those initializations. tcpdump calls nd_init() early in main(). exit_tcpdump() is static to tcpdump.c; it calls nd_cleanup() and then exit(). Other places that exited were either changed to call nd_cleanup() before exiting or changed to use ndo->ndo_error() rather than just printing a message and exiting.

[bijalthanawala]

Thanks @guyharris , @infrastation.

Given my beginner status in the project, reviewing your series of patches is beyond my knowledge at this point.
I would later, however, pull your commits and play with it to understand it, before moving to any other open issues or feature requests.
Please feel free to guide my efforts if you have any suggestions.

[infrastation]

Thank you Guy. @bijalthanawala, if you would like to study a useful feature request that is well documented but not implemented, try looking at #480, #415, #296 and the-tcpdump-group/libpcap#127.

[bijalthanawala]

Thanks @infrastation