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.
bluhm
