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.

Sign up for GitHub

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

Jump to bottom

Improve sorting and displaying algorithms for top-clients and top-domains #196

Merged
merged 6 commits into from
Jan 11, 2018
Merged

Improve sorting and displaying algorithms for top-clients and top-domains #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
15f5c2a
Implement improved algorithm in getTopDomains(). Instead of assuming …
DL6ER Jan 8, 2018
684f334
Implement improved algorithm in getTopClients(). As a by-product this…
DL6ER Jan 8, 2018
19a851c
Fix meaning of ascending and descending
DL6ER Jan 8, 2018
71434f8
Another descending (absteigend) and ascending (aufsteigend) tweak
DL6ER Jan 8, 2018
85cf6c6
Extend tests
DL6ER Jan 8, 2018
ff6258f
Fine-tune new tests
DL6ER Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions request.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,13 @@ void getTopDomains(char *client_message, int *sock)
{
char server_message[SOCKETBUFFERLEN];
int i, temparray[counters.domains][2], count=10, num;
bool blocked = command(client_message, ">top-ads"), audit = false, desc = false;
bool blocked = command(client_message, ">top-ads"), audit = false, asc = false;

// Exit before processing any data if requested via config setting
if(!config.query_display)
return;


// Match both top-domains and top-ads
if(sscanf(client_message, ">%*[^(](%i)", &num) > 0)
{
Expand All @@ -349,9 +350,9 @@ void getTopDomains(char *client_message, int *sock)
}

// Sort in descending order?
if(command(client_message, " desc"))
if(command(client_message, " asc"))
{
desc = true;
asc = true;
}

for(i=0; i < counters.domains; i++)
Expand All @@ -366,10 +367,10 @@ void getTopDomains(char *client_message, int *sock)
}

// Sort temporary array
if(desc)
qsort(temparray, counters.domains, sizeof(int[2]), cmpdesc);
else
if(asc)
qsort(temparray, counters.domains, sizeof(int[2]), cmpasc);
else
qsort(temparray, counters.domains, sizeof(int[2]), cmpdesc);


// Get filter
Expand Down Expand Up @@ -406,43 +407,40 @@ void getTopDomains(char *client_message, int *sock)
}
}

int skip = 0;
for(i=0; i < min(counters.domains, count+skip); i++)
int n = 0;
for(i=0; i < counters.domains; i++)
{
// Get sorted indices
int j = temparray[counters.domains-i-1][0];
int j = temparray[i][0];
validate_access("domains", j, true, __LINE__, __FUNCTION__, __FILE__);

// Skip this domain if there is a filter on it
if(excludedomains != NULL)
{
if(insetupVarsArray(domains[j].domain))
{
skip++;
continue;
}
}
if(excludedomains != NULL && insetupVarsArray(domains[j].domain))
Copy link
Member Author

Choose a reason for hiding this comment

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

Using short-circuit evaluation here, no functional change.

continue;

// Skip this domain if already included in audit
if(audit && countlineswith(domains[j].domain, files.auditlist) > 0)
{
skip++;
continue;
}

if(blocked && showblocked && domains[j].blockedcount > 0)
{
if(audit && domains[j].wildcard)
sprintf(server_message,"%i %i %s wildcard\n",i,domains[j].blockedcount,domains[j].domain);
sprintf(server_message,"%i %i %s wildcard\n", n, domains[j].blockedcount, domains[j].domain);
else
sprintf(server_message,"%i %i %s\n",i,domains[j].blockedcount,domains[j].domain);
sprintf(server_message,"%i %i %s\n", n ,domains[j].blockedcount, domains[j].domain);
swrite(server_message, *sock);
n++;
}
else if(!blocked && showpermitted && (domains[j].count - domains[j].blockedcount) > 0)
{
sprintf(server_message,"%i %i %s\n",i,(domains[j].count - domains[j].blockedcount),domains[j].domain);
sprintf(server_message,"%i %i %s\n", n, (domains[j].count - domains[j].blockedcount), domains[j].domain);
swrite(server_message, *sock);
n++;
}

// Only count entries that are actually sent and return when we have send enough data
if(n > count)
break;
}
if(excludedomains != NULL)
clearSetupVarsArray();
Expand Down Expand Up @@ -482,8 +480,18 @@ void getTopClients(char *client_message, int *sock)
temparray[i][1] = clients[i].count;
}

// Sort in ascending order?
bool asc = false;
if(command(client_message, " asc"))
{
asc = true;
}

// Sort temporary array
qsort(temparray, counters.clients, sizeof(int[2]), cmpasc);
if(asc)
qsort(temparray, counters.clients, sizeof(int[2]), cmpasc);
else
qsort(temparray, counters.clients, sizeof(int[2]), cmpdesc);

// Get clients which the user doesn't want to see
char * excludeclients = read_setupVarsconf("API_EXCLUDE_CLIENTS");
Expand All @@ -494,31 +502,30 @@ void getTopClients(char *client_message, int *sock)
logg("Excluding %i clients from being displayed", setupVarsElements);
}

int skip = 0;
for(i=0; i < min(counters.clients, count+skip); i++)
int n = 0;
for(i=0; i < counters.clients; i++)
{
// Get sorted indices
int j = temparray[counters.clients-i-1][0];
int j = temparray[i][0];
validate_access("clients", j, true, __LINE__, __FUNCTION__, __FILE__);

// Skip this client if there is a filter on it
if(excludeclients != NULL)
{
if(insetupVarsArray(clients[j].ip) ||
insetupVarsArray(clients[j].name))
{
skip++;
continue;
}
}
if(excludeclients != NULL && (insetupVarsArray(clients[j].ip) || insetupVarsArray(clients[j].name)))
Copy link
Member Author

Choose a reason for hiding this comment

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

Using short-circuit evaluation here, no functional change.

continue;

// Return this client if either
// - "withzero" option is set, and/or
// - the client made at least one query within the most recent 24 hours
if(includezeroclients || clients[j].count > 0)
{
sprintf(server_message,"%i %i %s %s\n",i,clients[j].count,clients[j].ip,clients[j].name);
sprintf(server_message,"%i %i %s %s\n", n, clients[j].count, clients[j].ip, clients[j].name);
swrite(server_message, *sock);
n++;
}

// Only count entries that are actually sent and return when we have send enough data
if(n > count)
break;
}
if(excludeclients != NULL)
clearSetupVarsArray();
Expand Down
44 changes: 37 additions & 7 deletions test/test_suite.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ load 'libs/bats-support/load'
[[ ${lines[11]} == "---EOM---" ]]
}

@test "Top Clients" {
@test "Top Clients (descending, default)" {
run bash -c 'echo ">top-clients" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
Expand All @@ -41,23 +41,53 @@ load 'libs/bats-support/load'
[[ ${lines[4]} == "---EOM---" ]]
}

@test "Top Domains" {
@test "Top Clients (ascending)" {
run bash -c 'echo ">top-clients asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} =~ "0 1 10.8.0.2" ]]
[[ ${lines[2]} =~ "1 2 127.0.0.1" ]]
[[ ${lines[3]} =~ "2 4 192.168.2.208" ]]
[[ ${lines[4]} == "---EOM---" ]]
}

@test "Top Domains (descending, default)" {
run bash -c 'echo ">top-domains" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 2 play.google.com" ]]
[[ ${lines[2]} == "1 1 example.com" ]]
[[ ${lines[2]} == "1 1 raspberrypi" ]]
[[ ${lines[3]} == "2 1 checkip.dyndns.org" ]]
[[ ${lines[4]} == "3 1 raspberrypi" ]]
[[ ${lines[4]} == "3 1 example.com" ]]
[[ ${lines[5]} == "---EOM---" ]]
}

@test "Top Domains (ascending)" {
run bash -c 'echo ">top-domains asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 raspberrypi" ]]
[[ ${lines[2]} == "1 1 checkip.dyndns.org" ]]
[[ ${lines[3]} == "2 1 example.com" ]]
[[ ${lines[4]} == "3 2 play.google.com" ]]
[[ ${lines[5]} == "---EOM---" ]]
}

@test "Top Ads" {
@test "Top Ads (descending, default)" {
run bash -c 'echo ">top-ads" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 addomain.com" ]]
[[ ${lines[2]} == "1 1 blacklisted.com" ]]
[[ ${lines[1]} == "0 1 blacklisted.com" ]]
[[ ${lines[2]} == "1 1 addomain.com" ]]
[[ ${lines[3]} == "---EOM---" ]]
}

@test "Top Ads (ascending)" {
run bash -c 'echo ">top-ads asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 blacklisted.com" ]]
[[ ${lines[2]} == "1 1 addomain.com" ]]
[[ ${lines[3]} == "---EOM---" ]]
}

Expand Down