On Tue, Jan 25, 2005 at 09:16:02AM +0500, Boris Kovalenko wrote: > Hello! > > >by this specific implementation. I'm sure we can keep an interface that > >handles priorities as seperate interfaces, but I'm not sure we'll want > >to do it via the vlan device (attractivly simple though that is.) > > > >This patch appears to be against 4 or 5. In 6 we've largly rewritten > >ifconfig so the patch won't apply. It looks like a simple matter to fix > >this issue. We'll need to commit to 6 before 4 or 5. > > > >I've embeded some comments in the text below. > Ok, so what I should do now? Rewrite patch for 6?
Let's get the version for 5 looking good first since we should be able
to MFC. We can do the ifconfig part for 6 once that's clean.
> >>+ if(tag < 1 || tag > 4094)
> >>+ errx(1, "VLAN ID shoud be in range 1..4094");
> >
> >
> >errx should be fully indented.
> What this means? What difference between my errx and this one (from 6)?
> errx(1, "must specify both vlan tag and device");
err is indented with a tab and four spaces. It should be indented with
two tabs like this:
if(tag < 1 || tag > 4094)
errx(1, "VLAN ID shoud be in range 1..4094");
> >I know other nearby code does this, but atoi should not be used. It has
> >not useful error checking. strtoul should be used instead.
> No problem.
> >> */
> >>struct vlanreq {
> >>- char vlr_parent[IFNAMSIZ];
> >>- u_short vlr_tag;
> >>+ char vlr_parent[IFNAMSIZ];
> >>+ u_int16_t vlr_tag;
> >
> >
> >This appears to be a no-op. Is it needed?
> Hmm... just to clarify that vlr_tag is 16bit value. If this is
> unnecessary I may use u_short.
Assuming the code compiles cleanly, let's leave it a u_short in 5. We
can change it to a u_int16_t in 6 since I agree that's more precise.
Changes to types, even ones that obviously don't do anything tend to
make re@ concerned about possible ABI breakage so I'd rather not worry
them. I tend to do that enough with my other projects. :)
-- Brooks
--
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
pgppW6b9OFHAJ.pgp
Description: PGP signature
