On 17/08/15(Mon) 19:24, Alexander Bluhm wrote:
> On Mon, Aug 17, 2015 at 07:03:55PM +0200, Martin Pieuchot wrote:
> > On 17/08/15(Mon) 18:25, Alexander Bluhm wrote:
> > > On Mon, Aug 17, 2015 at 12:34:13PM +0200, Martin Pieuchot wrote:
> > > > Ultimately my goal is to use rt_ifa_{add,del}() instead of
> > > > nd6_prefix_{on,off}link() but right now I need to remove the
> > > > rt->ref_cnt--.
> > >
> > > > @@ -1861,26 +1856,11 @@ nd6_prefix_onlink(struct nd_prefix *pr)
> > > > info.rti_info[RTAX_NETMASK] = sin6tosa(&mask6);
> > > >
> > > > error = rtrequest1(RTM_ADD, &info, RTP_CONNECTED, &rt,
> > > > ifp->if_rdomain);
> > > > - if (error == 0) {
> > > > - if (rt != NULL) /* this should be non NULL, though */
> > > > - rt_sendmsg(rt, RTM_ADD, ifp->if_rdomain);
> > > > + if (error == 0 && rt != NULL) {
> > > > pr->ndpr_stateflags |= NDPRF_ONLINK;
> > >
> > > Here you change the check for setting ndpr_stateflags from (error
> > > == 0) to (error == 0 && rt != NULL). Although I think that both
> > > checks have the same result, would it be better to use the same
> > > logic as in defrouter_addreq()?
> >
> > I'm changing the logic to match what's done in rt_ifa_add() but if you
> > look at rtrequest1(RTM_ADD, ...) you'll see that rt cannot be NULL when
> > error is 0.
> >
> > I think it is be better to have the same check everywhere and I don't
> > care if it is (error != 0), (rt == NULL) or both.
>
> I also want to have the same logic everywhere, it is quite inconsistent
> now.
>
> I think the best would be to check only for (error == 0). Then it
> is clear that rtrequest1() guarantees rt != NULL. The man page
> rtrequest1(9) should mention this.
>
> If we agree upon this, I can make a diff to unify the caller.
I agree on this, I'd love to see a diff to unify this behavior.
