Wireshark-bugs: [Wireshark-bugs] [Bug 1179] cmake build integration + dbus + bunch of memleak fi
Date: Sun, 5 Nov 2006 00:33:02 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1179


[email protected] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |WONTFIX




------- Comment #25 from [email protected]  2006-11-05 00:33 GMT -------
After looking (again) more than another two hours at your remaining patch and
the "fixes" ...

Your fixes are ONLY fixes of bugs you've introduced yourself by your own code.
I'm about to reverting them all, as they don't make any sense and will only
complicate and slow down the loop code (which is really performance relevant -
to not drop packets while capturing!).

capture_loop.c - by design - doesn't do any checks but will depend that ALL
given capture_opts parameters are valid (the caller - dumpcap.c/tshark must
ensure this). By injecting sniffer_dbus_main() at the wrong place you've simply
produced a lot of mess.

Example of a wrong fix (capture_loop.c line 246), you've added the "if(pipename
== NULL)" statement:

  g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "cap_pipe_open_live: %s",
pipename);

  if(pipename == NULL){
    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_WARNING, "cap_pipe_open_live:
pipe name is NULL");
    g_snprintf(errmsg, errmsgl,
               "The capture session could not be initiated because the pipe is
not valid, maybe lack of privileges?");
    return -1;
  }

The mistakes:
- IF pipename IS really NULL, on Win32 this will crash already in the call to
g_log() (Win32 don't like %s filled with NULL)
- the pipename string is completely untested (caused by your injected code) -
so the problems may be caused by a variety of reasons

The real solution:
Prevent cap_pipe_open_live() from being called with a pipename==NULL.

Conclusion: EVERY fix I've looked at is caused by your misunderstanding of the
code implications, not by any mistakes in the code itself!!!

AFAIK: in your d-bus code you've used g_mutex which won't work on Win32 and you
use the term sniffer which is trademarked by Network Associates ...

Thanks for taking a lot of my time by throwing a lot of code at us without just
asking first how to do it right ...

Your code therefore is buggy by design and should not be included in the
dumpcap  sources. A solution might be to convert capture_loop/dumpcap into a
library and use it by a separate dumpcap-dbus (or alike) program (which we then
don't have to maintain here). But that would require a better design discussed
at the mailing list first.


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.