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

[Alexander Eremin]

On Jun 9, 2011, at 9:48 PM, Dan McDonald wrote:

> 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.

Yep, that's better.

>
> 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.
>

You're right, thanks a lot  Dan.


> Thanks,
> Dan
>
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer

:: Alexander