On Thu, 27 Aug 2009 16:48:27 -0700
Renee Danson Sommerfeld <renee.sommerfeld at sun.com> wrote:
> On Thu, Aug 27, 2009 at 03:09:20PM -0700, Michael Hunter wrote:
> > /net/coupe.eng/builds/mph/nwam1_work/
> >
> > Changes cover:
> > 10456 automatic: wired and wireless up
> > 10605 lost state changes
> > collapse door_switch logging
> >
> > Please review.
>
> A few comments, mostly nits.
>
> -renee
>
> events.c
> 743: a comma after 'can' will make this more readable
>
> 752-53: I know 'rc' does not imply a number to everyone, but it does to me,
> and *I* find this code confusing as a result. Maybe a more
> descriptive name for this boolean would be a compromise: something
> like 'selected'?
ACCEPTED
>
> 759,769: Doesn't cstyle want spaces around the '+'?
It didn't barf on this code. But I added them in since it makes it
easier for some to read the code. Some might not like my new choice on
759 though...
>
> ncp.c
> 394-95: If this is going to stay in, the latter three parameters should
> be converted to strings.
ACCEPTED (removed)
>
> 550: A comment explaining what this function does would be helpful.
ACCEPTED
>
> 557-58: This message is a little misleading, as we're really deactivating
> all groups of lower or equal priority to the one given.
ACCEPTED
>
> 578: Spaces around '+'?
ACCEPTED
>
> 582: s/Returns if it.../Returns 'true' if it.../
ACCEPTED
>
> ncu.c
> 1211: This debug message is a little cryptic...should either remove it or
> make it more specific.
ACCEPTED (removed)
>
> ncu_phys.c
> 1759: Indeed! Need to resolve the XXX somehow...
If I figure it out I'll change it but I really think Alan needs to give
us some insight.
New webrev at /net/coupe.eng/builds/mph/nwam1_work
mph
>
> >
> > mph
> > _______________________________________________
> > nwam-dev mailing list
> > nwam-dev at opensolaris.org
> > http://mail.opensolaris.org/mailman/listinfo/nwam-dev
