Re: Looking at the windows 64 sockets allowed bug.

[James Rogers <jamesmrogers () gmail com>]

Things will work very badly if the defined value ever gets out of sync
between header files.  When I was researching this one of the
recommended ways was to have a project define, just as you suggest.
I defined it in the project at first because I was not aware of how
strangely interconnected the header files in the project have grown.
Obviously there needs to be some serious refactoring of the code base
to allow clearer interfaces and cleaner header files, something well
beyond the scope of a summer of code project.

The only problem I see with having FD_SETSIZE defined inside the
project is that it becomes invisible, set from something that isn't
easily seen or found.  Maybe also have a comment in the main windows
header file that explains where the FD_SETSIZE comes from?  I'll try
just setting this value in the project and see how that work.

The max function used with magic number of 5 that is used whenever the
max number of file handle functions available and the -5 magic number
used to subtract out the already used could all be pulled back into
this one function with a clear explanation of why these magic numbers
are being returned.  Having duplicated code in multiple places, all
associated with the same function is just begging for the duplicated
code to be pulled back into the function.

I am looking at
http://msdn.microsoft.com/en-us/library/ms724451%28v=vs.85%29
GetVersionEx() to try to tell different versions of windows apart at
runtime, so I can either return FD_SETSIZE or 5 if we are on the
socket limited version of windows.

On Tue, Jul 17, 2012 at 11:16 AM, David Fifield <david () bamsoftware com> wrote:
> On Tue, Jul 10, 2012 at 10:37:52AM +0200, Henri Doreau wrote:
> > 2012/7/9 James Rogers <jamesmrogers () gmail com>:
> > > I've done some benchmarking.  With the -sT option on 32bit Windows 7
> > > I can scan in 138 seconds  compare to 392 seconds  in nmap 6.01.
> > > This is almost a 3 times speed up.   Just to be through, on an
> > > unpatched nmap 6.02 the -sT option runs in 362 seconds.   An example
> > > of this comparison is in -sTtestnmap.jpg that I have attached to this
> > > email.
> > >
> > > I don't have another version of windows with which to test.  If
> > > someone could try out this patch on their own computer, that would be
> > > good. I wouldn't want to add something that breaks ore reduces
> > > performance on older versions of the windows build.
> > >
> > > I tried a normal scan as well, and noticed that the normal scan was
> > > taking at least 3 times longer to complete.  To make sure this was not
> > > caused by the above patch I rechecked out and compiled and in Nmap
> > > 6.02
> > >
> > > I am seeing 12 second scan times for a single host in the latest Nmap
> > > 6.02 compared to 3 to 4 second scan times in Nmap 6.01.  An example of
> > > this comparison is in NormalNmap.jpg that I have attached to this
> > > email.
> > >
> > > So it looks like scan times have regressed between 6.01 and 6.02 on
> > > windows 7 - 32 bit.
> >
> > good to see this brought to the surface, thanks for your work.
> >
> > I'm a bit confused though, your FD_SETSIZE patch improves
> > performances, but tests showed an unrelated regression between 6.01
> > and current SVN trunk, right?
> >
> > If so, this issue should be reported in a different thread on the list.
> >
> > Beside this I have a suggestion concerning the implementation. Would
> > it be doable to have this new FD_SETSIZE set as a single top-level
> > #define in a windows specific header? This would let windows users
> > who're willing to recompile it with a higher FD_SETSIZE value to do it
> > very easily.
> >
> > e.g.:
> > file: mswin32/winclude.h
> > #define NMAP_MAXFD_WIN32    1024
> >
> > all the other files:
> > #define FD_SETSIZE    NMAP_MAXFD_WIN32
>
> I think this idea is fine, but I agree with Henri that there can't be a
> bunch of copies of a magic number in all the source files.
>
> Perhaps this is something that could be handled at the level of the
> Visual C++ project? The *.vcxproj files have a PreprocessorDefinitions
> element that looks like it can be used to define a symbol project-wide.
> Doing it this way (through the Visual C++ properties dialog) could be
> more robust than doing it in every source file.
>
> What happens if one source file uses a different value of FD_SETSIZE
> than another? Will anything break? This is guaranteed to happen in the
> future if we can't find a way to avoid defining FD_SETSIZE in every file
> that uses it.
>
> David Fifield
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/