Re: [Enhancement]Microsoft Version Table

[Gyanendra Mishra <anomaly.the () gmail com>]

Hi,
Thanks for the prompt  feedback :)

You should not write arrays like this. The integer indexing is automatic
> if you leave the indices off. The way it is written now, it is easy to
> make a mistake and skip an index, and hard to verify that every index is
> there with no duplicates.

I'll remove the integer indexing.

It's better to use #windows than a literal "70". If you use "70", you
> have to remember to update it whenever the windows table changes.
> If get_windows_version returns multiple values, then it should return an
> array, not a string separated by newlines.

I realized that hard coding 70 was wrong and made the change. Also I am
returning tables now in a new
version I haven't yet posted.

The typeof argument doesn't appear to be used?

I realized that and classified each and every product as 'Server' or
'Client' in the newer version.Do we need to have more types?

 This is not a good way of handling errors. You can document that
>
minversion and majversion are required, and just crash if they are not

provided. That is a better outcome that if "give majorversion" appears

in script output somewhere.

I'll change that and return a nil and in  say smb-mbenum I'll put a check
on what the get_windows_version() function
returns.

You cannot put interactive code like this in a library. You can never
> read from the keyboard like this. This code is going to run whenever
> someone imports your library, making it useless for other scripts to
> use.
> Rather than the interactive code, you should try enhancing the
> smb-mbenum script to import your library and use the get_windows_version
> function. Use that as a demonstration of the new functionality.

I had put the interactive code just to show the basic working of the code
I'll remove it now.
I'll restart working on extending the smb-mbenum script.I tried to extend
that but was getting '445 filtered'
on most of the hosts I could try the script on. Also I'll change
get_windows_version() to get_windows_longname()
among other things.

Regards,
Gyanendra

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