Skip to content

Commit

Permalink
Use the musl in-tree getopt_long() everywhere
Browse files Browse the repository at this point in the history
Besides the obvious limitation of being unavailable on Windows,
the standard is vague about getopt() and getopt_long() has many
non-portable pitfalls and buggy implementations, that increase
the maintainance cost a lot. Also the GNU libc code currently
in the tree is not suited for embedding and is unmaintainable.

Own maintainership for getopt_long() and use the musl implementation
everywhere. This way we don't need to worry if optreset is available,
or if the $OPERATING_SYSTEM version behaves in subtly different ways.

The API is under the Wireshark namespace to avoid conflicts with
system headers.

Side-note, the Mingw-w64 9.0 getopt_long() implementation is buggy
with opterr and known to crash. In my experience it's a headache to
use the embedded getopt implementation if the system provides one.
  • Loading branch information
randstr committed Sep 16, 2021
1 parent 7462e76 commit 8df2a73
Show file tree
Hide file tree
Showing 41 changed files with 631 additions and 2,396 deletions.
23 changes: 0 additions & 23 deletions ConfigureChecks.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -93,29 +93,6 @@ if (NOT WIN32)
check_function_exists("clock_gettime" HAVE_CLOCK_GETTIME)
endif (NOT WIN32)

check_function_exists("getopt_long" HAVE_GETOPT_LONG)
if(HAVE_GETOPT_LONG)
check_include_file("getopt.h" HAVE_GETOPT_H)
#
# The OS has getopt_long(), so it might have optreset.
# Do we have it?
#
if(HAVE_GETOPT_H)
check_symbol_exists("optreset" "getopt.h" HAVE_OPTRESET)
else()
check_symbol_exists("optreset" HAVE_OPTRESET)
endif()
else()
#
# The OS doesn't have getopt_long(), so we're using the GNU libc
# version that we have in wsutil. It doesn't have optreset, so we
# don't need to check for it.
#
# However, it uses alloca(), so we may need to include alloca.h;
# check for it.
#
check_include_file("alloca.h" HAVE_ALLOCA_H)
endif()
check_function_exists("getifaddrs" HAVE_GETIFADDRS)
check_function_exists("issetugid" HAVE_ISSETUGID)
check_function_exists("setresgid" HAVE_SETRESGID)
Expand Down
19 changes: 4 additions & 15 deletions capinfos.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,7 @@
#include <locale.h>
#include <errno.h>

/*
* If we have getopt_long() in the system library, include <getopt.h>.
* Otherwise, we're using our own getopt_long() (either because the
* system has getopt() but not getopt_long(), as with some UN*Xes,
* or because it doesn't even have getopt(), as with Windows), so
* include our getopt_long()'s header.
*/
#ifdef HAVE_GETOPT_LONG
#include <getopt.h>
#else
#include <wsutil/wsgetopt.h>
#endif
#include <wsutil/ws_getopt.h>

#include <glib.h>

Expand Down Expand Up @@ -1620,7 +1609,7 @@ main(int argc, char *argv[])
wtap_init(TRUE);

/* Process the options */
while ((opt = getopt_long(argc, argv, "abcdehiklmnoqrstuvxyzABCDEFHIKLMNQRST", long_options, NULL)) !=-1) {
while ((opt = ws_getopt_long(argc, argv, "abcdehiklmnoqrstuvxyzABCDEFHIKLMNQRST", long_options, NULL)) !=-1) {

switch (opt) {

Expand Down Expand Up @@ -1803,7 +1792,7 @@ main(int argc, char *argv[])
}
}

if ((argc - optind) < 1) {
if ((argc - ws_optind) < 1) {
print_usage(stderr);
overall_error_status = INVALID_OPTION;
goto exit;
Expand All @@ -1825,7 +1814,7 @@ main(int argc, char *argv[])

overall_error_status = 0;

for (opt = optind; opt < argc; opt++) {
for (opt = ws_optind; opt < argc; opt++) {

(void) g_strlcpy(file_sha256, "<unknown>", HASH_STR_SIZE);
(void) g_strlcpy(file_rmd160, "<unknown>", HASH_STR_SIZE);
Expand Down
2 changes: 1 addition & 1 deletion capture_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ capture_opts_cleanup(capture_options *capture_opts);

/* set a command line option value */
extern int
capture_opts_add_opt(capture_options *capture_opts, int opt, const char *optarg);
capture_opts_add_opt(capture_options *capture_opts, int opt, const char *ws_optarg);

/* log content of capture_opts */
extern void
Expand Down
15 changes: 2 additions & 13 deletions captype.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,7 @@
#include <locale.h>
#include <errno.h>

/*
* If we have getopt_long() in the system library, include <getopt.h>.
* Otherwise, we're using our own getopt_long() (either because the
* system has getopt() but not getopt_long(), as with some UN*Xes,
* or because it doesn't even have getopt(), as with Windows), so
* include our getopt_long()'s header.
*/
#ifdef HAVE_GETOPT_LONG
#include <getopt.h>
#else
#include <wsutil/wsgetopt.h>
#endif
#include <wsutil/ws_getopt.h>

#include <glib.h>

Expand Down Expand Up @@ -157,7 +146,7 @@ main(int argc, char *argv[])
wtap_init(TRUE);

/* Process the options */
while ((opt = getopt_long(argc, argv, "hv", long_options, NULL)) !=-1) {
while ((opt = ws_getopt_long(argc, argv, "hv", long_options, NULL)) !=-1) {

switch (opt) {

Expand Down
6 changes: 0 additions & 6 deletions cmakeconfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
/* Define if you have the 'getexecname' function. */
#cmakedefine HAVE_GETEXECNAME 1

/* Define to 1 if you have the getopt_long function. */
#cmakedefine HAVE_GETOPT_LONG 1

/* Define to 1 if you have the <grp.h> header file. */
#cmakedefine HAVE_GRP_H 1

Expand Down Expand Up @@ -217,9 +214,6 @@
/* Define to 1 if you have the <pwd.h> header file. */
#cmakedefine HAVE_PWD_H 1

/* Define to 1 if you have the optreset variable */
#cmakedefine HAVE_OPTRESET 1

/* Define to 1 if you want to playing SBC by standalone BlueZ SBC library */
#cmakedefine HAVE_SBC 1

Expand Down
9 changes: 9 additions & 0 deletions debian/libwsutil0.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,9 @@ libwsutil.so.0 libwsutil0 #MINVER#
ws_buffer_remove_start@Base 1.99.0
ws_cleanup_sockets@Base 3.1.0
ws_cmac_buffer@Base 3.1.0
ws_getopt@Base 3.5.1
ws_getopt_long@Base 3.5.1
ws_getopt_long_only@Base 3.5.1
ws_buffer_cleanup@Base 2.3.0
ws_hexstrtou16@Base 2.3.0
ws_hexstrtou32@Base 2.3.0
Expand Down Expand Up @@ -387,6 +390,12 @@ libwsutil.so.0 libwsutil0 #MINVER#
ws_logv_full@Base 3.5.0
ws_mempbrk_compile@Base 1.99.4
ws_mempbrk_exec@Base 1.99.4
ws_optarg@Base 3.5.1
ws_opterr@Base 3.5.1
ws_optind@Base 3.5.1
ws_optopt@Base 3.5.1
ws_optpos@Base 3.5.1
ws_optreset@Base 3.5.1
ws_pipe_close@Base 2.6.5
ws_pipe_data_available@Base 2.5.0
ws_pipe_init@Base 2.5.1
Expand Down
41 changes: 15 additions & 26 deletions dumpcap.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,7 @@
#include <netinet/in.h>
#endif

/*
* If we have getopt_long() in the system library, include <getopt.h>.
* Otherwise, we're using our own getopt_long() (either because the
* system has getopt() but not getopt_long(), as with some UN*Xes,
* or because it doesn't even have getopt(), as with Windows), so
* include our getopt_long()'s header.
*/
#ifdef HAVE_GETOPT_LONG
#include <getopt.h>
#else
#include <wsutil/wsgetopt.h>
#endif
#include <wsutil/ws_getopt.h>

#if defined(__APPLE__) && defined(__LP64__)
#include <sys/utsname.h>
Expand Down Expand Up @@ -5126,7 +5115,7 @@ main(int argc, char *argv[])
global_capture_opts.capture_child = capture_child;

/* Now get our args */
while ((opt = getopt_long(argc, argv, OPTSTRING, long_options, NULL)) != -1) {
while ((opt = ws_getopt_long(argc, argv, OPTSTRING, long_options, NULL)) != -1) {
switch (opt) {
case 'h': /* Print help and exit */
show_help_header("Capture network packets and dump them into a pcapng or pcap file.");
Expand Down Expand Up @@ -5166,7 +5155,7 @@ main(int argc, char *argv[])
case 'I': /* Monitor mode */
#endif
case LONGOPT_COMPRESS_TYPE: /* compress type */
status = capture_opts_add_opt(&global_capture_opts, opt, optarg);
status = capture_opts_add_opt(&global_capture_opts, opt, ws_optarg);
if (status != 0) {
exit_main(status);
}
Expand All @@ -5177,7 +5166,7 @@ main(int argc, char *argv[])
interface_options *interface_opts;

interface_opts = &g_array_index(global_capture_opts.ifaces, interface_options, global_capture_opts.ifaces->len - 1);
interface_opts->ifname = g_strdup(optarg);
interface_opts->ifname = g_strdup(ws_optarg);
} else {
cmdarg_err("--ifname must be specified after a -i option");
exit_main(1);
Expand All @@ -5188,7 +5177,7 @@ main(int argc, char *argv[])
interface_options *interface_opts;

interface_opts = &g_array_index(global_capture_opts.ifaces, interface_options, global_capture_opts.ifaces->len - 1);
interface_opts->descr = g_strdup(optarg);
interface_opts->descr = g_strdup(ws_optarg);
} else {
cmdarg_err("--ifdescr must be specified after a -i option");
exit_main(1);
Expand All @@ -5198,19 +5187,19 @@ main(int argc, char *argv[])
if (capture_comments == NULL) {
capture_comments = g_ptr_array_new_with_free_func(g_free);
}
g_ptr_array_add(capture_comments, g_strdup(optarg));
g_ptr_array_add(capture_comments, g_strdup(ws_optarg));
break;
case 'Z':
capture_child = TRUE;
#ifdef _WIN32
/* set output pipe to binary mode, to avoid ugly text conversions */
_setmode(2, O_BINARY);
/*
* optarg = the control ID, aka the PPID, currently used for the
* ws_optarg = the control ID, aka the PPID, currently used for the
* signal pipe name.
*/
if (strcmp(optarg, SIGNAL_PIPE_CTRL_ID_NONE) != 0) {
sig_pipe_name = g_strdup_printf(SIGNAL_PIPE_FORMAT, optarg);
if (strcmp(ws_optarg, SIGNAL_PIPE_CTRL_ID_NONE) != 0) {
sig_pipe_name = g_strdup_printf(SIGNAL_PIPE_FORMAT, ws_optarg);
sig_pipe_handle = CreateFile(utf_8to16(sig_pipe_name),
GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);

Expand Down Expand Up @@ -5260,7 +5249,7 @@ main(int argc, char *argv[])
case 'k': /* Set wireless channel */
if (!set_chan) {
set_chan = TRUE;
set_chan_arg = optarg;
set_chan_arg = ws_optarg;
run_once_args++;
} else {
cmdarg_err("Only one -k flag may be specified");
Expand All @@ -5271,22 +5260,22 @@ main(int argc, char *argv[])
machine_readable = TRUE;
break;
case 'C':
pcap_queue_byte_limit = get_positive_int(optarg, "byte_limit");
pcap_queue_byte_limit = get_positive_int(ws_optarg, "byte_limit");
break;
case 'N':
pcap_queue_packet_limit = get_positive_int(optarg, "packet_limit");
pcap_queue_packet_limit = get_positive_int(ws_optarg, "packet_limit");
break;
default:
cmdarg_err("Invalid Option: %s", argv[optind-1]);
cmdarg_err("Invalid Option: %s", argv[ws_optind-1]);
/* FALLTHROUGH */
case '?': /* Bad flag - print usage message */
arg_error = TRUE;
break;
}
}
if (!arg_error) {
argc -= optind;
argv += optind;
argc -= ws_optind;
argv += ws_optind;
if (argc >= 1) {
/* user specified file name as regular command-line argument */
/* XXX - use it as the capture file name (or something else)? */
Expand Down
Loading

0 comments on commit 8df2a73

Please sign in to comment.