Hi Jack, Thanks for looking at this.
* Jack Schwartz ([email protected]) wrote: > Hi Glenn. > > Nice job at managing all of this. Here are my comments for the > first code review: > > beadm.py: > > Both gettext.install() calls are removed. Isn't one of them > (or maybe _.install()) needed? You're correct. I added back the original gettext.install() calls. > be_list.c: > > zone_be_contianer_ds is a typo in many places. Fixed. > be_mount.c: > > 1745: Can get rid of the lint error by applying DeMorgan's law and > inverting all operators. This is cleaner than just putting /* > LINTED */ to cover up the issue. > > !(a && b && (c || d)) > !a || !b || !(c || d) > !a || !b || (!c && !d) > > if (p1 != NULL && > strncmp(p1 + 1, BE_CONTAINER_DS_NAME, 4) == 0 && > /* LINTED statement has no consequent */ > (*(p1 + 5) == '/' || *(p1 + 5) == '\0')) { > ; > } else { > else_stuff(); > } > > becomes > > if (p1 == NULL || > strncmp(p1 + 1, BE_CONTAINER_DS_NAME, 4) != 0 || > (*(p1 + 5) != '/' && *(p1 + 5) != '\0')) { > else_stuff(); > } Changed as suggested. > be_utils.c: > > 2304/2305: can be condensed to: > (void) vfprintf(stderr, prnt_str, ap); Existing code, and I'd rather not change it. > libbe.h: > > It makes more sense to me to put lines 29-31 below the #include > lines of 33-35. Each #include line brings in the contents of a > file, which itself will have > #ifdef __cplusplus. > Nesting these #ifdefs (through the includes) seems like > unnecessarily asking for trouble. Changed as suggested. > libbe_priv.h: > > Same comment as for libbe.h Changed as suggested. > tbeadm.c: > > How come tbeadm.c is getting deleted? Is it being replaced by the > official tests done by Harold? This is being added back in after some discussion with various people. Thanks Jack! Glenn _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
