Re: [NSE] Locals, Globals, and You (ALL script writers please read)

[Ron <ron () skullsecurity net>]

For what it's worth, and I realize it isn't necessary (and maybe technically wrong in Lua), I like declaring variables 
at the top as a matter of style. Even in languages where I don't have to, declaring variables at the top feels cleaner 
and gives me a better idea of what's going on in the function than declaring them willy nilly where I use them. 

I realize I do a mixture of both when I'm writing Lua, and that's bad -- but it's purely because of a) laziness, and b) 
my functions get too long (which is a problem in an of itself). 

One thing I run into is situations where I use the same variable (like status) multiple times, but use a different 
variable alongside. For example:
status, variable1 = function1()
status, variable2 = function2()
status, variable3 = function3()

There doesn't seem to be a clean way to do that, unless it's okay to re-declare status as local:
local status, variable1 = function1()
local status, variable2 = function2()
...

That's when I start declaring stuff at the top of the function. It gets even worse when I need to use the return value 
of 'status' in a call to the next function, because then you're re-declaring it and using it on the same line and 
that's all kinds of not good. 

I know you have a script that'll find accidental global accesses. And that's good, because I've spent a lot of time 
tracking down nasty bugs that are caused by exactly that (accidental globals). But is there any way to make it easier 
to run your script? Like, even if it can't be automated (nmap --script-check-globals?), at least including it in Nmap 
or something? I wish Lua wouldn't even let you declare a global within a function. I like Ruby's style better, of using 
@@blah for a global, to totally differentiate it. 

On a related note, I'm pretty sure there's currently no "style guide" for writing scripts, and I'm sure that if you 
took the 5 people who write the most scripts, none of them would agree on proper styles (even things like whitespace, 
comments, variable declarations, etc). But maybe we should start looking into building a style guide. 

Ron

On Sat, 14 Aug 2010 21:53:26 -0400 Patrick Donnelly
<batrick () batbytes com> wrote:
> I'll keep this as brief as possible because I know few of you care
> about the idosyncracies of Lua and would prefer writing awesome NSE
> scripts!
>
> A recent disturbing trend I've been seeing in libraries and scripts is
> people declaring locals at the top of their functions (seemingly,
> because they're used to that in C). There are other problems I've seen
> as well, such as trying to reuse locals thinking it saves them space.
> I hope to convince everyone to stop doing this and help you adopt
> better Lua practices.
>
> Quick example of what I'm talking about to illustrate my point (from
> smb.lua, please don't hate me Ron!):
>
> function get_socket_info(host)
>     local status, lhost, lport, rhost, rport
>     local smbstate, socket
>
>     -- Start SMB (we need a socket to get the proper local ip
>     status, smbstate = smb.start_ex(host)
>     if(status == false) then
>         return false, smbstate
>     end
>
>     socket = smbstate['socket']
>     status, lhost, lport, rhost, rport = socket:get_info()
>         if(status == false) then
>         return false, lhost
>     end
>
>     -- Stop SMB
>     smb.stop(smbstate)
>
>     -- Get the mac in hex format, if possible
>     local lmac = nil
>     if(host.mac_addr_src) then
>         lmac = stdnse.tohex(host.mac_addr_src, {separator = ":"})
>     end
>
>     return true, lhost, lport, rhost, rport, lmac
> end
>
> As you can see, most of the locals were declared upfront instead of
> where the assignments appear.
>
>
> == Local and Global ==
>
> A *little* bit of highly condensed information on globals and locals.
> Lua has local variables that exist on the stack. These variables
> persist only in the scope they were declared. Local variables are
> initialized to nil unless another value is set. Local variables are
> faster than global variable access (think stack access versus heap).
> You can read more about local variables from the Roberto
> Ierusalimschy's (Lua architect) Programming in Lua [2]. It's a *short*
> section I recommend reading through. (For even more relevant sections,
> see [3] and [4].)
>
> Global variables are the default in assignments or accesses. If you do
> not declare a variable of the given name as local, then Lua will check
> for a global instead. Globals are shared by *all* scripts and
> libraries (with some exceptions). This means they can be overwritten
> unintentionally by other unrelated libraries or scripts. A user should
> avoid using globals in favor of locals.
>
> =======================
>
> Ok, on to an example of some problems most everyone is having with
> locals (from nselib/http.lua):
>
> local request_line, header, body
> request_line = string.format("%s %s HTTP/1.1", method, path)
> header = {}
> local name, value
> for name, value in pairs(mod_options.header) do
>   header[#header + 1] = string.format("%s: %s", name, value)
> end
> body = mod_options.content and mod_options.content or ""
>
> do this instead:
>
> local request_line = string.format("%s %s HTTP/1.1", method, path)
> local header = {}
> for name, value in pairs(mod_options.header) do
>   header[#header + 1] = string.format("%s: %s", name, value)
> end
> local body = mod_options.content and mod_options.content or ""
>
>
> There are a couple reasons why:
>
>
> (1) Perhaps *most importantly* it helps prevent accidental global
> accesses (see [1] for more information on this). I believe most of the
> accidents where a library writer uses a global instead of a local is
> because they forgot to add the local at the beginning of their
> function. (Hey it's common in C too right! How often do you get an
> undeclared variable *compiler* error?) Lua won't complain if you use
> an undeclared variable; it will just use a global instead! So an error
> like this:
>
> local socket, _, foo, bar, etc;
> -- ... code
> socket, error = nmap.socket();
>
> is quite common! Note that 'error' is not declared a local! Instead
> Lua will assign to the *global* variable 'error'.
>
> (2) Proper scoping, you want your variable's lifetime to be as short
> as possible. This assists in garbage collection and readability.
>
> (3) There is *no* benefit to declaring a variable you plan to reuse in
> multiple blocks up front. That is, if you plan to use a socket
> variable in a bunch of loops, re-declare the socket variable in each
> block as needed. There is *zero* extra cost if you do this. Lua is
> smart enough to reuse stack space.
>
> (4) Well, you'll save time coding of course! You don't have to go back
> to the top of your function to add 'x' new local.
>
> EXCEPTION: Don't redeclare a variable local when one already exists in
> the same block, e.g.:
>
>     status, err      = smb.negotiate_protocol(smbstate, overrides)
>     if(status == false) then
>         stdnse.print_debug("SMB; is_admin: Failed to negotiatie
> protocol: %s [%s]", err, username)
>         smb.stop(smbstate)
>         return false
>     end
>     status, err      = smb.start_session(smbstate, overrides)
>     if(status == false) then
>         stdnse.print_debug("SMB; is_admin: Failed to start session %s
> [%s]", err, username)
>         smb.stop(smbstate)
>         return false
>     end
>
>
> This snippet of code appeared in smb.lua. The same "status, err     =
> ...." statement appears dozens of times in the function. You should
> only declare the first "status, err" local.
>
> == For Loops ==
>
> Also, please note that a generic for loop automatically creates local
> variables for you:
>
> for i, v in ipairs(t) do ... end
>
> The variables 'i' and 'v' are automaticatically generated and local to
> the scope of "do ... end" block. They are not visible outside the
> scope of the for loop. You do *NOT* need to do this:
>
> local i,v
> for i, v in ipairs(t) do ... end
>
> ^ DO NOT DO (as in the first example) ^
>
>
> If you have any questions I'd be happy to answer them. Sorry the email
> wasn't very neat. I'm brain dumping most of this quickly for lack of
> time.
>
>
> [1] http://seclists.org/nmap-dev/2009/q3/70
> [2] http://www.lua.org/pil/4.2.html
> [3] http://www.lua.org/pil/4.1.html
> [4] http://www.lua.org/pil/6.1.html
>
>
> -- 
> - Patrick Donnelly
> _______________________________________________
> Sent through the nmap-dev mailing list
> http://cgi.insecure.org/mailman/listinfo/nmap-dev
> Archived at http://seclists.org/nmap-dev/


-- 
Ron Bowes
http://www.skullsecurity.org
http://www.twitter.com/iagox86

Attachment: _bin
Description:

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