[Xorp-cvs] XORP cvs commit: xorp/bgp

Pavlin Radoslavov pavlin@icir.org
Fri, 11 Nov 2005 09:56:28 -0800

> On Friday 11 November 2005 18:06, Pavlin Radoslavov wrote:
> > Presumably, the new aggregate-prefix-len inside the BGP policy
> > template will be hooked with the bgp_varrw.{hh,cc} modifications.
> > According to the recent modification to bgp.tp the variable is
> > registered as "aggregate_prefix_len" with the policy.
> > In that case, I believe the new BGPVarRW methods must match the
> > variable name and must be named read_aggregate_prefix_len() and
> > write_aggregate_prefix_len() instead of read_aggr_pref_len() and
> > write_aggr_pref_len() respectively (Andrea please correct me if this
> > is not the case).
> I tested the bgp_varrw interface with the latest modifications from 
> Andrea and it seems to work OK as is.  Whether there is a new naming 
> convention that needs to be followed is another story, I am / was not 
> aware of that.  I'll change the naming of the funcions as you suggest 
> if needed, but I'd prefer not to have exceedingly long method names if 
> not absolutely necessary.

On second thought, I think you are right. Initially I had brain
failure on trying to understand how exactly the policy code matches
a policy variable like "aggregate_prefix_len" introduced by some of
the rtrmgr templates with its corresponding read_aggr_pref_len() and
write_aggr_pref_len() methods. I simply overlooked the fact that the
VAR_FOO enum values inside class BGPVarRW actually do match the
values introduced by the add_varmap XRLs.

Nevertheless, given that the rest of the policy code uses same
naming scheme for the variable, for the enum, and the rw methods,
for the sake of consistency I'd recommend that you use similar
naming even if it means longer method names.