[frr] The bgpafisafi pull request

[Philippe Guibert]

Hello Donald,

I understood that the way this function was used, was like the other vty
conversion functions.
- get afi/safi values
- get afi/safi/vrf values.
meaning that the caller of the function must ensure that the values  he
passes as params have good initial values.

I would have done a little change.
This is because you handle pointers in that function (*vrf), and you assume
that this pointer is valid. Whereas an user that does not want to care
about VRF, does not want to initialise a specific value. he uses NULL as
parameter.
For that, I would have ignored vrf case ( returned 0 if a vrf is found, and
return ok if no vrf is found).

The rest of the series is ok for me.

Regards,

Philippe



On Thu, Jan 26, 2017 at 3:21 PM, Donald Sharp <sharpd at cumulusnetworks.com>
wrote:

> https://github.com/freerangerouting/frr/pull/107
>
> Philippe -
>
> You had a review comment two days ago:
>
> how do you initialise vrf in the case vrf_name is already null (
> commands with no vrf at all)?
> I would lie the *vrf value to be set inside that function.
>
> I responded with:
>
> I do not believe this is the right thing to do. Only the calling
> function knows what the right defaults are for the afi/safi/vrf due to
> we assume different defaults based upon the command. This approach
> would break the ability for the calling function to do the right
> thing. As I stated I will update the Pull Request with better
> documentation for this funciton.
>
> I've also added a commit that updates the function to further explain
> it.  Is there anything else you would like to have done?
>
> donald
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.frrouting.org/pipermail/dev/attachments/20170126/804e9379/attachment.html>