search

ericpaulbishop / gargoyle

Gargoyle Router Management Utility
http://www.gargoyle-router.com
468 stars 221 forks source link

qosmon: Support IPv6 #908

Closed lantis1008 closed 3 years ago

lantis1008 commented 3 years ago

Leaving this open for a few days to allow @pbix to comment if/as needed.

Switched parts of qosmon (mostly pinger/pr_pack related) to use more generic ip family agnostic data structures. Also dropped a few variables and operations that didn't seem to serve any purpose (probably left over debugging lines from long ago).

pbix commented 3 years ago

I did look at your work lantis. If you can mention which variables or operations you dropped I will comment on those. I did not seem them right off in the diff.

I understand your goal of adding IPv6 support to Gargoyle. Its a very ambitious undertaking I think and touches almost every aspect of the design. I am not willing to do it myself but wish you the best.

By inspection your code looks good but only testing can ensure your success.

lantis1008 commented 3 years ago

Thanks for looking! Sure thing, to be specific (referencing the old version to make line numbers easier): https://github.com/ericpaulbishop/gargoyle/blob/master/package/qos-gargoyle/src/qosmon.c Dropped L107-108 because they were only referenced by L791,L797. I suspect this was old debug code to print the hostname once loaded into the variable. It is never used elsewhere in the code. Dropped L55-L57 because they were only referenced by L107-108.

Dropped L344,L350 because they are never referenced outside the function. Dropped L336 because they were only referenced by L344,L350. Dropped L338 as the from variable is never really used. It is just a shell populated by recvfrom and passed into pr_pack. We actually could remove this from pr_pack altogether on reinspection?

pbix commented 3 years ago

Have no issues with these modifications.

As a bit of history I stole the pr_pack() function from the original ping() program (circa 1983).

https://nsrc.org/wrc/materials/src/ping.c

So any unused variables in the function are a relic of that.

Paul

On 1/2/21 8:59 PM, Michael wrote:

Thanks for looking! Sure thing, to be specific (referencing the old version to make line numbers easier): https://github.com/ericpaulbishop/gargoyle/blob/master/package/qos-gargoyle/src/qosmon.c Dropped L107-108 because they were only referenced by L791,L797. I suspect this was old debug code to print the hostname once loaded into the variable. It is never used elsewhere in the code. Dropped L55-L57 because they were only referenced by L107-108.

Dropped L344,L350 because they are never referenced outside the function. Dropped L336 because they were only referenced by L344,L350. Dropped L338 as the from variable is never really used. It is just a shell populated by recvfrom and passed into pr_pack. We actually could remove this from pr_pack altogether on reinspection?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ericpaulbishop/gargoyle/pull/908#issuecomment-753555883, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACGXQKY2LYE2VCEYVQVMVLSX7FRHANCNFSM4VQYUKVQ.