Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

local dns cache for statistics plugin #5

Merged
merged 2 commits into from
Mar 3, 2012
Merged

Conversation

jhamb
Copy link
Contributor

@jhamb jhamb commented Mar 2, 2012

added a local cache for resolved hosts in the statistics plugin to save time.

@@ -857,7 +857,14 @@ function extendedVisitorStatistics($max_items){
echo "<td class = \"".$color."\">".wordwrap($row['browser'], 25, "<br />", 1)."</td>\n";
echo "<td class = \"".$color."\">";
if ($row['ip']){
echo wordwrap(gethostbyaddr($row['ip']), 25, "\n", 1);
$curIP = $row['ip'];
if (array_key_exists($curIP, $resolvedHosts)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has $resolvedHosts been declared before? Otherwise this will generate a notice (at least in PHP 5.3+). Apart from that collecting duplicate IPs seems to be a good idea ...

@jhamb
Copy link
Contributor Author

jhamb commented Mar 3, 2012

Sorry, missed the declaration line. Added it and also expanded my tabs to spaces to match the indentation of the rest of the file. I think it's okay to cache on per request basis. Or do you think we should hold the hosts cached for a bigger scope?

@garvinhicking
Copy link
Member

Great! Many thanks for the contribution!

(Man, I love github pull requests! This is sooo easy)

garvinhicking added a commit that referenced this pull request Mar 3, 2012
local dns cache for statistics plugin
@garvinhicking garvinhicking merged commit e79176a into s9y:master Mar 3, 2012
@mattsches
Copy link
Member

github pull requests are full of win :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants