Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: Match ICMP echo reply to request in scan_engine.cc
From: Chris Johnson <cjohnson () zenoss com>
Date: Fri, 09 Aug 2013 09:27:52 -0400 (EDT)

http://git.io/nmap-cjohnson-3.diff (and attached)

I've updated the patch again according to your feedback:
 * Created and used struct IPExtraProbeData_icmp, rather than directly on the probe.
 * Moved declaration of struct ppkt to scan_engine.cc.

I wasn't sure where to put some declarations, as far as ordering within the file. Feel free to move things.

 - chris

----- Original Message -----
From: "David Fifield" <david () bamsoftware com>
To: "Chris Johnson" <cjohnson () zenoss com>
Cc: dev () nmap org
Sent: Thursday, August 8, 2013 3:18:52 PM
Subject: Re: Match ICMP echo reply to request in scan_engine.cc

On Thu, Aug 08, 2013 at 04:00:37PM -0400, Chris Johnson wrote:
2. I disagree with the suggestion of storing icmp_ident in
probespec_icmpdata. The ident value is (ideally) unique to each probe.

It's fine to say that, but that's not how it works in fact. In the
patch, the ident member is used by no other probe type than ICMP.
UltraProbe could use a refactoring to have a unique ID per probe, like
Probe::token in traceroute.cc. But doing so shouldn't be a part of this
patch. It's not good to add a new class member that will be
uninitialized except in the ICMP case.

But now that I look at it more, I think a better place to store the ICMP
ID is in a new struct IPExtraProbeData_icmp, like the existing
IPExtraProbeData_tcp, IPExtraProbeData_udp, and IPExtraProbeData_sctp.

3. I've created an icmp_probe_match helper function, as suggested.

Thanks, that looks good. Please move the declaration of struct ppkt to
scan_engine.cc if it doesn't need wider scope.

David Fifield

Attachment: nmap-cjohnson-3.diff

Sent through the dev mailing list
Archived at http://seclists.org/nmap-dev/

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]