Hi Intrigeri, don't worry about the delay. I think you had all your hands full with GSoC. Still it does have the disadvantage of me moving to a new town to start university in a week or so, which means I am cut off from a computer where I can test the script on a live TAILS system (I do have some other testscripts, however). So I'm hoping I didn't introduce any stupid but easy to debug bugs :-(
None the less: Diff as always is attached. And here are my comments to your comments: > Had you used Memoize (https://metacpan.org/module/Memoize), which I > still recommend since it's exactly the right tool for the task at > hand, you could just: > return $i2p_default_port; > ... which expresses much more clearly your intentions I think. > You are the boss. I switched it to Memoize. > It is very non-idiomatic to first read the whole file into a > multi-line string, then apply a regex on it to extract a sub-string. > > A more idiomatic way to do it is: > > my $regex = > '^clientApp\.([0-9]+)\.main=net\.i2p\.router\.web\.RouterConsoleRunner$'; > while ( my $line = <$conffile> ) { > chomp $line; > if (($client_app_no) = ($line =~ m/$regex/)) { > } The problem I am trying to solve here is that the second Regex actually is(or can be) appearing one or multiple lines _before_ the first regex in the file, but I need the regex in the correct order, otherwise I can't compose the second RegEx. So I am facing the choice of reading/caching the file completely or reading it twice from HDD (relying upon another cache grabbing it (Linux kernel, ...). But that seems undesirable and slower). + I could try doing the second RegEx first, but I'd have to store the potential values in an array and get the right one later. That just seemed wrong. + I could store the whole file in an array of strings instead of a single string, but I think it will be slower, as the Perl developers probably spent more time optimizing their code than is being realistic for me. And I have to put the file in a variable anyway... + I could make a completely new function just to return the file content and use Memoize on it.But that seems like overkill... Am I missing something here? > > + $regex = '\nclientApp\.' . $client_app_no . '\.args=([0-9]{1,5})\ > > [0-9s:,. \-]+ \.\/webapps\/[\s]*\n'; > > Regarding [0-9s:,. \-], I still doubt you really want to include the > "\" character in the caracter class. OK. Finally got rid of it ;-) ) > Also, preferably use \s or [[:space:]], or even [[:space:]]+, instead > of "\ ". It's both clearer to read, and likely to be more robust on > the long run especially when when parsing files you don't control. > The "+" I suggest also allows you to remove the space in the character > class that follows. changed it to [[:space:]] . However the space in the following characterclass is still needed, as we might encounter something like: "$PORT$ ::1,127.0.0.1 -s 7667 ::1,127.0.0.1 ./webapps/" ^ ^ ^ ^ which has multiple additional spaces inside our character class match Arguably the [[:space:]] before and after the character class could be dropped but I thought it adds to clarity. As thus I did not put a "+" , as the additional spaces will be caught anyway, and the extra ones are really only for the sake of the reader. (However, whatever you prefere, shall be done :) > Regarding "[\s]*", if you mean "possibly followed by space(s)", I > think "[[:space:]]*" is clearer as it does not require the reader to > remember if \s is interpreted or not inside a character class. I have changed it to "[[:space:]]" however according to my tests it can actually be followed by TABs as well. (Probably won't. Doesn't seem likely someone mistakenly puts a TAB there) Cheers, Cryptkiddy
diff --git a/config/chroot_local-includes/usr/local/bin/tails-start-i2p b/config/chroot_local-includes/usr/local/bin/tails-start-i2p index 716d05d..659d122 100755 --- a/config/chroot_local-includes/usr/local/bin/tails-start-i2p +++ b/config/chroot_local-includes/usr/local/bin/tails-start-i2p @@ -25,6 +25,8 @@ See https://tails.boum.org/. use Desktop::Notify; use Locale::gettext; use POSIX; +use Memoize; +memoize('get_router_port'); ### initialization setlocale(LC_MESSAGES, ""); @@ -32,9 +34,44 @@ textdomain("tails-i2p-notify-user"); ### helper subs -# TODO: get router port (default 7657) from /etc/i2p/clients.config sub get_router_port { - return 7657; + # gets router port from /etc/i2p/clients.config . If this fails we return I2P default port (7657). + my $i2p_default_port = 7657; + my $router_port = 0; + + + my $conffile; + if(! open($conffile, "<", "/etc/i2p/clients.config")) { + warn "Can not open i2p config file '/etc/i2p/client.config'.". + " Using default port ".$i2p_default_port." for I2P webinterface instead."; + return $i2p_default_port; + } + my $client_config = join("", <$conffile>); + close($conffile); + + my $client_app_no; + my $regex = '\nclientApp\.([0-9]+)\.main=net\.i2p\.router\.web\.RouterConsoleRunner\n'; + #we are trying to get $NUMBER$ from >>clientApp.$NUMBER$.main=net.i2p.router.web.RouterConsoleRunner<< + if(! (($client_app_no) = ($client_config =~ m/$regex/))) { + warn "RegEx checking 'clientApp.[0-9].main=net.i2p.router.web.RouterConsoleRunner' didn't match.". + "Can not find port in '/etc/i2p/client.config'." . + " Using default port ".$i2p_default_port." for I2P webinterface instead."; + return $i2p_default_port; + } + + $regex = '\nclientApp\.' . $client_app_no . '\.args=([0-9]{1,5})[[:space:]]([0-9s:,.-]|[[:space:]])+[[:space:]]\.\/webapps\/[[:space:]]*\n'; + # 'clientApp.$NUMBER$.args=$PORT$ ::1,127.0.0.1 ./webapps/' + # $PORT$ ::1 ./webapps/ + # $PORT$ ::1,127.0.0.1 -s 7667 ::1,127.0.0.1 ./webapps/ + #here $NUMBER$ is the one we obtained above and $PORT$ is what we want to get + #doesn't match if only https port is given. + if (! (($router_port) = ($client_config =~ m/$regex/))) { + warn "RegEx checking 'clientApp.".$client_app_no.".args= [...]' didn't match.". + "Can not find port in '/etc/i2p/client.config'." . + " Using default port ".$i2p_default_port." for I2P webinterface instead."; + return $i2p_default_port; + } + return $router_port; } # TODO: more perlish way to do below?
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ tails-dev mailing list [email protected] https://mailman.boum.org/listinfo/tails-dev
