-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
ospfd: fix state location mixup #16361
ospfd: fix state location mixup #16361
Conversation
In the "2x2 matrix" of these, I accidentally edited "row-wise" when I should've edited "column-wise"... *sigh* Reported-by: github.com/rbfnet Fixes: FRRouting#16349 Fixes: 110945b ("ospfd: fix GR state location") Signed-off-by: David Lamparter <[email protected]>
|
||
/* this one includes the path... because the instance number was in the path | ||
* before :( ... which totally didn't have a mkdir anywhere. | ||
* |
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.
I don't see problems with the C logic but I'd rather see a comment explaining the usage and rationale behind the various paths. Your comment as written seems only useful in the context of what you are changing as opposed to what paths are created and what they are to be used for.
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.
done (but I've put it in lib/libfrr.h
because the variables are declared there & used beyond ospfd)
It's not immediately obvious what exactly the `frr_*dir` variables exported from lib/libfrr.c are for. Add a little text each to clarify. Signed-off-by: David Lamparter <[email protected]>
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.
Looks good see one question.
OSPFD_COMPAT_STATE_NAME); | ||
/* no COMPAT2 here since it was reading that was broken, |
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.
What does "it was reading that was broken" mean?
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.
looks good
In the "2x2 matrix" of these, I accidentally edited "row-wise" when I should've edited "column-wise"... sigh
Reported-by: @rbfnet
Fixes: #16349
Fixes: 110945b ("ospfd: fix GR state location")