-
Notifications
You must be signed in to change notification settings - Fork 1.1k
IPTraffic: get hostnames from static DHCP list #1136
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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){ |
There was a problem hiding this comment.
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(">"); |
There was a problem hiding this comment.
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(/>/g, ">").replace(/</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:
nvram_char_to_ascii
callnvram_char_to_ascii
function associationej_nvram_char_to_ascii
definitionnvram_safe_get_x
callnvram_safe_get_x
definition /nvram_get_x
call
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.
The function clientFromIP is only used in the files:
originData.staticList contains a list of the static DHCP ip adresses and mac adresses, i use this to make a lookup in the table.