Closed wgc1212 closed 5 years ago
Good catch! No, that change was not intentional. The commit that introduced this was only supposed to change from a uint32_t
based bitmask to use a variable length array.
Do you think you could help out with a fix and submit as a pull request?
Sure! I'm working the code now, I just need a good way to test it.
Bill
On Thu, Nov 15, 2018 at 2:49 AM Joachim Nilsson notifications@github.com wrote:
Good catch! No, that change was not intentional. The commit that introduced this was only supposed to change from a uint32_t based bitmask to use a variable length array.
Do you think you could help out with a fix and submit as a pull request?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/troglobit/pimd/issues/131#issuecomment-438948439, or mute the thread https://github.com/notifications/unsubscribe-auth/Aq4LbnJeRWqUg70Vv6g5fkADmfxDe12Cks5uvRySgaJpZM4YfNx- .
I wasn't sure how updates to the project were supposed to work. Should I enter a pull request for the code first? The routine I was thinking of would be as follows. This would check for any non-zero leaves and oifs in the arrays:
inline static int PIMD_VIFM_LASTHOP_ROUTER(uint8_t *leaves, uint8_t *oifs)
{
int i=0;
for (; ((i<MAXVIFS) && (!leaves[i] || !oifs[i])); ++i);
return((i<MAXVIFS) ? 1 : 0);
}
A pull request is always preferable, they work well also for prototyping code together or getting feedback.
The code looks OK, but I'd like to propose the following instead, which I hope is more readable and yet still correct:
inline static int PIMD_VIFM_LASTHOP_ROUTER(uint8_t *leaves, uint8_t *oifs)
{
int i;
for (i = 0; i < MAXVIFS; i++) {
if (leaves[i] && oifs[i])
return 1;
}
return 0;
}
What do you think?
That looks fine. I tried to do a pull request, and I didn't seem to be allowed to.
Anybody can do a pull request against pimd. There are no limitations in place. Have you read up on how to do it? https://help.github.com/articles/creating-a-pull-request-from-a-fork/
Briefly, the process is as follows:
Thanks. I have gone through the GitHub process. I had gotten a couple of steps out of order. I have updated the code and have submitted the Pull request.
Good work! Small final comment from me.
https://github.com/troglobit/pimd/blob/3aefd816a49b6065daad9de8222d708ba7571f12/src/vif.h#L82
Comparing PIMD_VIFM_LASTHOP_ROUTER() to the released comparison for last hop check:
define VIFM_LASTHOP_ROUTER(leaves, oifs) ((leaves) & (oifs))
The released software comparison will accept any interface that has both a leave and an oif, while the new comparison requires all interfaces to have both set in order to qualify as a Lasthop router. Is this assessment correct and was this change intentional?
Thanks.