Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 6645] Patch to add support for Windows Friendly Interface Names

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6645

Mike Garratt <wireshark@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7524|0                           |1
        is obsolete|                            |
   Attachment #7524|review_for_checkin?         |
              Flags|                            |

--- Comment #3 from Mike Garratt <wireshark@xxxxxxxxx> 2012-09-24 06:05:53 PDT ---
Created attachment 9218
  --> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=9218
wireshark_windows_interface_friendly_names_patch_ v2_1.9.0rev45085

Okay, well the followup patch wasn't a few days as planned.  The patch has been
working great for me, things got busy at work so till now things have sat on
the back burner.

Attached is an updated patch against subversion revision 45085.
I included the changes as suggested around standardisng the use of quotation
marks between OS versions etc.

I explored pushing the description for the Interface Friendly Name earlier in
the code logic - this would mean changing at least the pcap_findalldevs()
function which right now like most of capture-wpcap.c in a pure wrapper of a
winpcap call.  I am unsure if additional changes would be required to ensure
the correct behaviour around interface name lookups or if such a change would
have an unintended impact elsewhere.

I have reviewed the amount of change in the attached updated patch (which
doesnt change pcap_findalldevs(), I think the patch may have made the
appearance that a lot of files are being changed for the interface friendly
name functionality... I don't think it is all that different from a change to
pcap_findalldevs() based on the following:

* I still believe it is cleaner to keep the ~170 lines of code dealing with
retrieving interface friendly names from the OS, and lookups etc in
capture_win_ifnames.c + capture_win_ifnames.h.
* 5 of the files in the patch relate to this separation:
   capture_win_ifnames.c 
   capture_win_ifnames.h.
   cmakelists.txt - tweak to compile capture_win_ifnames.c file
   Makefile.am - tweak to compile capture_win_ifnames.c file
   Makefile.nmake - tweak to compile capture_win_ifnames.c file

* dumpcap.c - the patch introduces logic on windows to display the interface
friendly name if found and falling back to the device name when unfound.  If
the description switch was pushed into 
pcap_findalldevs() this fallback wouldn't be possible without adding more
complex code/tracking of unfound names.  Further dumpcap.c would still be
getting edits to add quotes around interface name and the fflush(stderr) call.

Pushing description swapping into pcap_findalldevs() would remove the need for
the following files to be changed:
* capture_opts.c - wouldn't need the 9 lines of code
* capture-pcap-util.c - wouldn't need the 18 lines of code
* capture-pcap-util-int.h - wouldn't need the 1 line change to make "char *
description" parameter in if_info_new() const
This is weighed up against doing the change in pcap_findalldevs().

After reviewing the above, I sit in the camp that the current approach is
appropriate.  I don't personally have the time right now to evaluate, implement
and test a change into pcap_findalldevs() and other potential locations (such
as lookup functions and other pcap functions to enumerate interfaces etc).  The
current approach works great as is.

I think the patch is a high value improvement (as shown by the screen dump I am
about to attach).

I would be interested to hear the thoughts of the development team.

Thanks for your consideration,

- Mike
-

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.