Skip to content
This repository has been archived by the owner on Dec 14, 2020. It is now read-only.

IPTraffic: get hostnames from static DHCP list #1136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zone31
Copy link

@zone31 zone31 commented Dec 6, 2016

The function clientFromIP is only used in the files:

  • Main_TrafficMonitor_devdaily.asp
  • Main_TrafficMonitor_devrealtime.asp
  • Main_TrafficMonitor_devmonthly.asp

originData.staticList contains a list of the static DHCP ip adresses and mac adresses, i use this to make a lookup in the table.

Copy link

@Thynix Thynix left a comment

Choose a reason for hiding this comment

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

This implementation looks good to me, but it'd be nice to add a little documentation while we're at it.

@@ -3263,7 +3263,18 @@ function oui_query_web(mac){
function clientFromIP(ip) {
for(var i=0; i<clientList.length;i++){
var clientObj = clientList[clientList[i]];
if(clientObj.ip == ip) return clientObj;
if(clientObj.ip == ip){
Copy link

Choose a reason for hiding this comment

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

This mixes a formatting change with a functional change. This is small enough that it doesn't strike me as a problem to have it in the same commit, but seems good to avoid in the future.

staticList = originData.staticList;
for(var i=0; i<staticList.length; i++){
if(staticList[i] != ""){
data = staticList[i].split(">");
Copy link

@Thynix Thynix Dec 30, 2016

Choose a reason for hiding this comment

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

Thoughts on commenting here (or better yet, here?) what the expected format is? I had to go scrounging to find that data[0] is [0] the MAC address and that clientList is indeed keyed by MAC address.

How about: [1]

/* Entries in dhcp_staticlist are formatted as: "MAC>IP>hostname". */
staticList: decodeURIComponent('<% nvram_char_to_ascii("", "dhcp_staticlist"); %>').replace(/&#62/g, ">").replace(/&#60/g, "<").split('<'),

It could also be nice (though I don't feel too strongly about it) to set var static_ip and var static_mac or something from data instead of using them inline and leaving their values more implicit.

EDIT: The bigger picture here is that staticList should perhaps contain objects, not strings that must be split each time. That's definitely a separate change though.

Other than that this LGTM.


[0] Probably? I was able to find nvram_char_to_ascii but not the implementation that reads from nvram. Is it in a binary somewhere? It seems like the larger need is documentation of what's in nvram and in what format, but I haven't run into that yet.

I was able to trace thusly:

but found no definition for nvram_get_x().

[1] assuming the nvram value matches that in index.asp - the behavior of which suggests this is dumped straight into nvram which seems incredibly fragile.

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

Successfully merging this pull request may close these issues.

2 participants