Ethereal-dev: Re: [Ethereal-dev] Patch for tethereal taps

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Guy Harris <guy@xxxxxxxxxx>
Date: Tue, 29 Oct 2002 12:15:50 -0800
On Tue, Oct 29, 2002 at 10:36:52AM -0500, Jason House wrote:
> I like the cleanup to tethereal, but would still prefer to see a slightly
> different implementation.  I'm thinking that tethereal should not know about
> tap_list and that the tap_listeners should not know about '-z' for tethereal.

I agree.

> consider replacing:
> for(tli=tap_list;tli;tli=tli->next){
>    if(!strncmp(tli->cmd,optarg,strlen(tli->cmd))){
>      (*tli->func)(optarg);
>      break;
>   }
> }
> 
> with something like:
> for (int i=0;i<strlen(optarg); i++){
>     if (optarg[i] == ','){
>         optarg[i] = '\0';
>         char *error_message = init_tap_listener( optarg, &optarg[i+1]  );
>         if (error_message != NULL){
>             printf("error using -z, %s config error: %s", optarg,
> error_message);
>         }
>         break;
>     }
> }

I'd do

	comma = strchr(optarg, ',');
	if (comma != NULL) {
		*comma = '\0';
		error_message = init_tap_listener(optarg, comma + 1);
		if (error_message != NULL) {
			fprintf(stderr, "tethereal: invalid -z argument: %s\n",
			    error_message);
			exit(1);
		}
	}

instead, as:

	1) you can just use "strchr()" to look for the comma;

	2) the error message isn't necessarily an error from a
	   particular tap listener, it might be an error saying there *is*
	   no such tap listener;

	3) neither

		for (int i = 0; ...) {
			...
		}

	   nor

		if (...) {
			xxx = yyy;
			char *zzz = www();
			...
		}

	   are legal C89.

>     A simple window where you select a tap listener from a combo box, and have
> one or two string inputs.

Or a mechanism by which tap listeners can be added to the "Tools" menu
(including a mechanism to add new submenus), and, when the menu item is
selected, have a dialog box popped up to provide input to the tap and an
optional filter expression.

>     I could also imagine that the taps could have a preference registration
> mechanism similar to the protocol dissectors.

Sounds good to me.  That way the dialog box wouldn't just have two
strings, it could have items specific to the tap.

> A function to convert the
> preferences into a tap listener configuration string would force tethereal to
> be at least as capable as ethereal...

I.e., to use the preferences to control how the stuff after the "," in
the argument to "-z" is parsed, so that the parsing is done *outside*
the tap itself?  Sounds good.

> I'm sure there can be plenty more to this
> discussion, for the moment a preference registration seems a bit of overkill
> with so few tap listeners... but maybe that's the whole reason to implement it
> now...

Yes, I think that is a reason to implement it now.  I'd like to see taps
uses as a very general extension mechanism for Ethereal, so people can
make add-on modules for graphing, computing statistics, doing "expert"
analysis, etc. (which is why I wanted to be able to just pass a protocol
tree to a tap; that way, you don't have to modify dissectors to provide
information that a tap would use, and you can write taps that do things
that the dissector writer didn't anticipate).