On Tue, 2009-08-04 at 13:47 -0700, Darren Reed wrote:
> On 4/08/09 01:17 PM, Sebastien Roy wrote:
> > Darren,
> >
> > On Mon, 2009-07-27 at 19:01 -0700, Darren Reed wrote:
> >
> > > http://cr.opensolaris.org/~darrenr/onnv-pcapture/
> > >
> > >
> >
> > Do you have a cscope database for this code-review repo somewhere to
> > help with browsing the code?
> >
>
> Is there a way to upload a cut-down version of that to opensolaris
> or does this remain a SWAN thing only?
cscope needs to co-exist with an actual repo on a filesystem. I suppose
you could post cscope databases on opensolaris in a repo and require
people to pull that repo to a local filesystem to use it...
>
> /net/nvtbld-x.sfbay/exportz/avalon/on_bpf_pfpacket_review/
>
Thanks. Could you run xref under usr/src/uts/ as well please?
> btw, in reviewing the code myself, I had some questions about if
> I should do some renaming in ipnet.c...
>
> For example, there is "ipnetif_refhold/ipnetif_refrele" but then
> there is "ipnet_create_if" and "ipnet_free_if". Also, in some places
> "ipnetif" is the "local" variable for an ipnetif_t and in others it is
> "ifp".
Feel free to fix those inconsistencies.
>
> Whilst seemingly minor, it doesn't feel like it is all the product
> of a single design?
It's not a design-level issue, but rather an implementation issue. It's
a product of having multiple people working on the file over time, and
perhaps not synchronizing totally on nomenclature.
> During working on that file, I got annoyed
> enough with the different code styles used for local variables
> that I converted them all to be one because otherwise it was not
> clear to which style new code should be added!
The style is, and should be (for multiple variables):
<type> <name>;
<type> <name>;
Where each name is aligned at a common tab-stop. I don't see many
inconsistencies here, I'm not sure what you're referring to. I'd
refrain from making sweeping changes to the code style in the file, as
that detracts from reviewing your actual substantive changes.
> And in review,
> I've come up against the same issue with adding new functions
> but it has bothered me less.
Okay,
-Seb
