Re: Match ICMP echo reply to request in scan_engine.cc

[Chris Johnson <cjohnson () zenoss com>]

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
Description:

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