Hi Evan.
Here are my code review comments:
libbe.h: OK
libbe_priv.h: OK
be_activate.c: OK
be_create.c: OK
be_list.c: OK
be_rename.c: OK
be_utils.c:
292+ : Missing be_orig_root_ds in header comment.
529: missing explanation
944: You may want to clarify here that a bootfs entry with no name is
still valid. Such cases are found on line 1005 and 1018-1021.
969: def_bootfs not returning NULL for this error condition, contrary to
the header comment.
1456: Do you mean the boot or grub menu, or the boot/grub menu? If the
former, then the comment can be misleading.
Nit: 1505-1506: can be condensed to
*entry = ent_num - 1;
2653: Is it misleding to say "external error" here? Though not likely,
is it possible that a programming error internal to this module caused
the fault?
3024, 3051 and 3074: Since these are private functions, please declare
them static.
3054: Can be optimized as:
return (! (strcmp((char *)be_get_default_isa(), name) == 0));
3081: This statement will always be true.
Thanks,
Jack
On 12/04/08 12:01, Evan Layton wrote:
> We need to get some code reviews for the SPARC support
> for SNAP.
>
> The changes were mainly in how the SPARC boot menu is
> handled compared to how the grub menu is handled.
>
> The changes cover the following CR's only:
> http://defect.opensolaris.org/bz/show_bug.cgi?id=4297
> http://defect.opensolaris.org/bz/show_bug.cgi?id=5401
> http://defect.opensolaris.org/bz/show_bug.cgi?id=5420
> http://defect.opensolaris.org/bz/show_bug.cgi?id=5604
>
> The webrev is available from:
> http://cr.opensolaris.org/~evanl/5420
>
> Unit testing for this has been completed however general
> testing for this is not yet complete. This testing is in
> progress and has shown no new issues thus far.
>
> Thanks!
> -evan
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
