[illumos-Developer] Review request for #923 bootadm tries to rebuild archives which it shouldn't

Dan McDonald danmcd at nexenta.com
Thu Jun 9 10:48:51 PDT 2011


On Thu, Jun 09, 2011 at 09:29:12PM +0400, Alexander Eremin wrote:
> 
> >>This fix makes bootadm not update archive in update_all
> >>if mounted directory is not BE.
> >>webrev: http://cr.opensolaris.org/~alhazred/i923/

<SNIP!>

> Thanks for reviewing,  could I get a second review please?

Two nits:

Lines 2930, 2944, and 2949:   Just return B_FALSE here.  It makes things a
			      bit more readable.

Lines 2960-61:  Did you pick strncmp() here on purpose?  If, for example,
		cur_be->be_root_ds is a truncated string of ds_path, it'll
		match.  Is that desired behavior?  If you want a precise
		string match, you can safely use strcmp() because you're
		guaranteed by internal data structure safety that
		cur_be->be_root_ds is terminated.  If you end up using
		strcmp(), though, you should comment about internal structure
		guarantees explicitly.  Some code auditors blanch at any use
		of the unguarded str*() functions, most of the time with
		good reason.

Thanks,
Dan



More information about the Developer mailing list