Re: [NSE][PATCH] string_ext library

["Patrick Donnelly" <batrick.donnelly () gmail com>]

On Fri, Sep 19, 2008 at 2:44 PM, Fyodor <fyodor () insecure org> wrote:
> On Fri, Sep 19, 2008 at 10:05:33PM +0200, Sven Klemm wrote:
> > Sven Klemm wrote:
> >
> > > i've attached a library that adds a to_hex function to string. I think
> > > this library would be a good place for adding further string
> > > enhancements like for example moving stdnse.strsplit() here.
> > >
> > > example usage:
> > >
> > > require "string_ext"
> > >
> > > ("abc"):to_hex() => "616263"
> > > ("abc"):to_hex(":") => "61:62:63"
> > > ("abc"):to_hex(":",2) => "6162:63"
>
> Hi Sven.  I think to_hex() sounds like great functionality to have!
> But I'd love to hear some feedback from the Lua experts around here on
> the idea of extending the string object rather than putting to_hex in
> our own namespace (such as stdnse or a new class for this purpose).
> Do we do this sort of standard object extension anywhere else in NSE?

No we do not.

> Adding to_hex to the string object does seem logical in some respects
> and makes for good-looking code.  But my concern is that it reduces
> the boundary between built-in Lua functionality and our own
> extensions, which may confuse readers.  And I'm not sure how well it
> will work with our upcoming NSEdoc web site.  It sort of reminds me of
> the way C++ allows operator overloading, which can make code look
> quite elegant, but is often considered a bad coding practice because
> it can confuse readers of the code.  But that isn't a perfect analogy.
>
> I agree that we should probably treat strsplit and to_hex the same
> way, whichever way that is.
>
> I'd be interested in hearing from the more Lua-experienced folks
> whether we should add to_hex() and strsplit() to string or create our
> own string/utility library for them.

Other applications (notably, World of Warcraft) often add to the
string library directly, such as string.strsplit. I recommended in [1]
that the string library be extended with our own library. You are
right this may confuse people new to Lua. I think it is fine for the
most part although people should be cautious extending libraries in
this way because it can easily lead to incompatibilities. But...

I should add that while this lends a certain elegance to indexing
string functions it will always be slower than just having a local
reference to this extension library and indexing the functions
directly.

e.g.

local string_ext = require"string_ext"

function action(host, port)
  print(string_ext.to_hex("abcd")) -- faster
  -- vs.
  print(("abcd"):to_hex()); -- slower
end

Beginning a practice where extended string functions are added to a
super class of the standard string library where speed is being
sacrificed will probably bite us in the long run. For this reason and
the confusion of new users, it's probably best to avoid doing this
(sorry Sven for having misled you earlier).

[1] http://seclists.org/nmap-dev/2008/q3/0794.html

Cheers,

-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org