Thanks,
The code is much more legible now. The split and extra check to see if the
result has the number of parts I expected is handy in case the output from
the OSX commands I use changes in the future.

I think I managed to incorporate all the remarks in the latest commit.

BR,
Peter



*Peter Toth*
*EPM Professional Ambassador *Sucaba Technologies AB, P.O Box 7735,
SE-10395 Stockholm, Sweden
Mobile: +46 708 997 333, peter.toth@sucaba.eu

ORACLE Excellence Award 2013
Specialized Partner of the Year Applications



[image: Sucaba Tech_RGB.png]

www.sucaba.eu




On Wed, Nov 13, 2013 at 1:47 PM, Josh <notifications@github.com> wrote:

> In modules/post/osx/manage/vpn.rb:
>
> > +        end
> > +      end
> > +    elsif action.name == 'CONNECT'
> > +      connect_vpn(true)
> > +    elsif action.name == 'DISCONNECT'
> > +      connect_vpn(false)
> > +    end
> > +  end
> > +
> > +  def get_vpn_list()
> > +    vprint_status(datastore['SCUTIL_PATH'].shellescape + " --nc list")
> > +    data = cmd_exec(datastore['SCUTIL_PATH'].shellescape + " --nc list")
> > +    return data
> > +  end
> > +
> > +  def parse_vpn_connection_names(data, show_up)
>
> this might make more sense as
>
> def parse_vpn_connection_names(data, type = :connected)
>   connection_names = []
>   # this will look for connected vpns when :connected is passed in, otherwise disconnected vpns
>   comp_str = type == :connected ? STR_CONNECTED : STR_DISCONNECTED
>   data.lines do |line|
>     connection_names << line.split('"')[1] if line.start_with?(comp_str)
>     # keep in mind my comments about checking the split to see that it worked the way you expected
>     # it may be ok to skip the split check if you know it's gonna work when the start_with works too
>     # instead of using start_with, you an also use a regex:  if line =~ /^#{comp_str}/
>     # regex is harder to read but they are very common and allow you to do much more complex checks and
>     # also allow you to easily do case insensitive checks with  if line =~ /^#{comp_str}/i
>   end
>   return connection_names
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/rapid7/metasploit-framework/pull/2632/files#r7624610>
> .
>