Wireshark · Ethereal-dev: Re: [ethereal-dev] UI Updates

[Guy Harris <gharris@xxxxxxxxxxxx>]

> Another possibility might be to pop up a dialog box for those errors; I
> have a patch to do that, although:
> 
> 	on UNIX/X, one might want to do that only if the standard output
> 	isn't a terminal, or if you're not running from a
> 	terminal/terminal emulator, or something such as that;

No, that's not sufficient - as I'd noted in an earlier discussion of
this

> 	1) there might not be a standard output to which to write it, or
> 	   the standard output might not go to some sensible place (it
>  	   might go to a text-mode console that is hidden while you're
>  	   in X, or might go to a console window that you might not be
> 	   watching);

and, in fact, given the way I currently start X on my machine (with
"startx", rather than with xdm/kdm/gdm), my KDE session, including KFM,
is attached to the virtual console from which I ran "startx", so if I
run Ethereal from a link on my desktop, its standard error is that
virtual console, which I normally see only after I exit X.

I've attached a patch that makes Ethereal pop up a usage message in a
message box as well as printing it to the standard error.  Given that
other errors from Ethereal that aren't the result of stuff done through
the UI (e.g., an error message due to being unable to read the
preferences file, or due to being unable to open a capture file
specified on the command line with "-r") show up only as message boxes,
it doesn't bother me too much to have Ethereal always pop up a message
box for command-line syntax errors (as "-r" on the command line, with a
bad file name as an argument to "-r", will also cause a message box to
pop up).

Index: gtk/main.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/gtk/main.c,v
retrieving revision 1.117
diff -c -r1.117 gtk/main.c
*** gtk/main.c	2000/04/14 09:00:25	1.117
--- gtk/main.c	2000/05/03 06:11:50
***************
*** 44,49 ****
--- 44,50 ----
  #include <stdlib.h>
  #include <stdio.h>
  #include <string.h>
+ #include <ctype.h>
  
  #ifdef HAVE_UNISTD_H
  #include <unistd.h>
***************
*** 1068,1094 ****
  	gtk_main_quit();
  }
  
! static void 
! print_usage(void) {
! 
!   fprintf(stderr, "This is GNU " PACKAGE " " VERSION ", compiled with %s\n",
! 	  comp_info_str);
  #ifdef HAVE_LIBPCAP
!   fprintf(stderr, "%s [ -vh ] [ -kQS ] [ -b <bold font> ] [ -B <byte view height> ]\n",
! 	  PACKAGE);
!   fprintf(stderr, "\t[ -c count ] [ -D ] [ -f <capture filter> ] [ -i interface ]\n");
!   fprintf(stderr, "\t[ -m <medium font> ] [ -n ] [ -P <packet list height> ] [ -r infile ]\n");
!   fprintf(stderr, "\t[ -R <read filter> ] [ -s snaplen ] [ -t <time stamp format> ]\n");
!   fprintf(stderr, "\t[ -T <tree view height> ] [ -w savefile ]\n");
  #else
!   fprintf(stderr, "%s [ -vh ] [ -b <bold font> ] [ -B <byte view height> ]\n",
! 	  PACKAGE);
!   fprintf(stderr, "\t[ -m <medium font> ] [ -n ] [ -P <packet list height> ] [ -r infile ]\n");
!   fprintf(stderr, "\t[ -R <read filter> ] [ -t <time stamp format> ]\n");
!   fprintf(stderr, "\t[ -T <tree view height> ]\n");
  #endif
  }
  
  /* And now our feature presentation... [ fade to music ] */
  int
  main(int argc, char *argv[])
--- 1069,1107 ----
  	gtk_main_quit();
  }
  
! static char usage_message[] = {
!   "%sThis is GNU " PACKAGE " " VERSION ", compiled with %s\n"
  #ifdef HAVE_LIBPCAP
!   "Usage: %s [ -vh ] [ -kQS ] [ -b <bold font> ] [ -B <byte view height> ]\n"
!   "\t[ -c count ] [ -D ] [ -f <capture filter> ] [ -i interface ]\n"
!   "\t[ -m <medium font> ] [ -n ] [ -P <packet list height> ] [ -r infile ]\n"
!   "\t[ -R <read filter> ] [ -s snaplen ] [ -t <time stamp format> ]\n"
!   "\t[ -T <tree view height> ] [ -w savefile ]\n"
  #else
!   "Usage: %s [ -vh ] [ -b <bold font> ] [ -B <byte view height> ]\n"
!   "\t[ -m <medium font> ] [ -n ] [ -P <packet list height> ] [ -r infile ]\n"
!   "\t[ -R <read filter> ] [ -t <time stamp format> ]\n"
!   "\t[ -T <tree view height> ]\n"
  #endif
+ };
+ 
+ /* Pop up a usage message, perhaps preceded by an error message, in a
+    window. */
+ static void
+ display_usage(char *error_msg)
+ {
+   simple_dialog(ESD_TYPE_WARN, NULL, usage_message, error_msg, comp_info_str,
+ 	PACKAGE);
  }
  
+ /* Print a usage message, perhaps preceded by an error message, to
+    the standard output. */
+ static void 
+ print_usage(char *error_msg)
+ {
+   fprintf(stderr, usage_message, error_msg, comp_info_str, PACKAGE);
+ }
+ 
  /* And now our feature presentation... [ fade to music ] */
  int
  main(int argc, char *argv[])
***************
*** 1100,1106 ****
--- 1113,1121 ----
    int                  i;
    int                  opt;
    extern char         *optarg;
+   extern int           optopt;
    gboolean             arg_error = FALSE;
+   GString             *opterrmsg;
  #ifdef HAVE_LIBPCAP
  #ifdef WIN32
    char pcap_version[] = "0.4a6";
***************
*** 1239,1246 ****
  #endif
     );
  
    /* Now get our args */
!   while ((opt = getopt(argc, argv, "b:B:c:Df:hi:km:nP:Qr:R:Ss:t:T:w:W:v")) != EOF) {
      switch (opt) {
        case 'b':	       /* Bold font */
  	bold_font = g_strdup(optarg);
--- 1254,1265 ----
  #endif
     );
  
+   /* Initialize the error message string.
+      Mallocate it, as we'll grow it as necessary. */
+   opterrmsg = g_string_new("");
+ 
    /* Now get our args */
!   while ((opt = getopt(argc, argv, ":b:B:c:Df:hi:km:nP:Qr:R:Ss:t:T:w:W:v")) != EOF) {
      switch (opt) {
        case 'b':	       /* Bold font */
  	bold_font = g_strdup(optarg);
***************
*** 1253,1258 ****
--- 1272,1280 ----
          cf.count = atoi(optarg);
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
          break;
***************
*** 1266,1276 ****
  	cf.cfilter = g_strdup(optarg);
  #else
          capture_option_specified = TRUE;
          arg_error = TRUE;
  #endif
  	break;
        case 'h':        /* Print help and exit */
! 	print_usage();
  	exit(0);
          break;
        case 'i':        /* Use interface xxx */
--- 1288,1301 ----
  	cf.cfilter = g_strdup(optarg);
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
  	break;
        case 'h':        /* Print help and exit */
! 	print_usage("");
  	exit(0);
          break;
        case 'i':        /* Use interface xxx */
***************
*** 1286,1291 ****
--- 1311,1319 ----
          start_capture = TRUE;
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
          break;
***************
*** 1304,1309 ****
--- 1332,1340 ----
          start_capture = TRUE;  /*** -Q implies -k !! ***/
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
          break;
***************
*** 1321,1326 ****
--- 1352,1360 ----
          cf.snap = atoi(optarg);
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
          break;
***************
*** 1329,1334 ****
--- 1363,1371 ----
          sync_mode = TRUE;
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
          break;
***************
*** 1359,1364 ****
--- 1396,1404 ----
          save_file = g_strdup(optarg);
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
  	break;
***************
*** 1367,1388 ****
          cf.save_file_fd = atoi(optarg);
  #else
          capture_option_specified = TRUE;
          arg_error = TRUE;
  #endif
  	break;
        default:
        case '?':        /* Bad flag - print usage message */
          arg_error = TRUE;
          break;
      }
    }
  
  #ifndef HAVE_LIBPCAP
!   if (capture_option_specified)
!     fprintf(stderr, "This version of Ethereal was not built with support for capturing packets.\n");
! #endif
!   if (arg_error)
!     print_usage();
  #ifdef HAVE_LIBPCAP
    if (start_capture) {
      /* We're supposed to do a live capture; did the user specify an interface
--- 1407,1466 ----
          cf.save_file_fd = atoi(optarg);
  #else
          capture_option_specified = TRUE;
+         g_string_sprintfa(opterrmsg,
+ "This version of Ethereal was not built with support for capturing packets,
+ so the -%c flag is not supported.\n", opt);
          arg_error = TRUE;
  #endif
  	break;
        default:
+         arg_error = TRUE;
+         if (opt >= 0 && opt < 256) {
+           g_string_sprintfa(opterrmsg, "-%c is not a valid flag.\n",
+ 				opt);
+ 	}
+         break;
        case '?':        /* Bad flag - print usage message */
          arg_error = TRUE;
+         /* Remember which flags had problems.
+ 	   XXX - the UNIX 98 "getopt()", if the option list string begins
+ 	   with a ":", will return ':' if the problem is that the option
+ 	   requires an argument but wasn't given one; we could use that
+ 	   to put up the right complaints in the message box, by recording
+ 	   which error character we got, *if* that behavior were universal,
+ 	   but I'm not sure it is. */
+         if (optopt >= 0 && optopt < 256) {
+           g_string_sprintfa(opterrmsg, "-%c is not a valid flag.\n",
+ 				optopt);
+ 	}
+         break;
+       case ':':        /* Bad flag - print usage message */
+         arg_error = TRUE;
+         /* Remember which flags had problems.
+ 	   XXX - the UNIX 98 "getopt()", if the option list string begins
+ 	   with a ":", will return ':' if the problem is that the option
+ 	   requires an argument but wasn't given one; we could use that
+ 	   to put up the right complaints in the message box, by recording
+ 	   which error character we got, *if* that behavior were universal,
+ 	   but I'm not sure it is. */
+         if (optopt >= 0 && optopt < 256) {
+           g_string_sprintfa(opterrmsg,
+ "The -%c flag requires an argument, but none was supplied.\n",
+ 				optopt);
+ 	}
          break;
      }
    }
  
  #ifndef HAVE_LIBPCAP
!   if (capture_option_specified) {
!     print_usage("This version of Ethereal was not built with support for capturing packets.\n");
!   } else
! #endif
!   {
!     if (arg_error)
!       print_usage("");
!   }
  #ifdef HAVE_LIBPCAP
    if (start_capture) {
      /* We're supposed to do a live capture; did the user specify an interface
***************
*** 1508,1513 ****
--- 1586,1606 ----
  #ifdef HAVE_LIBPCAP
    }
  #endif
+ 
+   /* If we had a command-line error, pop up an alert box, as the program
+      might not have been run from a terminal window, and any error message
+      that we printed may not show up; we defer it until now, so that the
+      alert box is more likely to come up on top of the main window.
+ 
+      We don't check whether the standard output is a tty, as it may well be
+      a tty even if it's not the terminal window from which it was run
+      (if you start an X session from the console of a PC or workstation,
+      the program from which you run Ethereal might be running with the
+      console as its standard output or error). */
+   if (arg_error) {
+     display_usage(opterrmsg->str);
+     g_string_free(opterrmsg, TRUE);
+   }
  
    /* If we failed to open the preferences file, pop up an alert box;
       we defer it until now, so that the alert box is more likely to