Evan Layton wrote:
> Tim Knitter wrote:
>> Please review the following bug fix:
>>
>> 5749 libbe to provide public interface to validate BE name
>>
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5749
>> http://cr.opensolaris.org/~tsk/5749/
>>
>> Thanks
>> Tim
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> 
> Hi Tim,
> 
> be_create.c:
> 208 to 213, 785 to 790 and 1973 to 1978 doesn't be_valid_be_name() already do 
> the check for a valid ZFS dataset name?
> 
> be_rename.c:
> 134 to 140 same comment as above.
> 

The current code only checked for the BE name. So only "bename" in the 
following dataset was checked:
 rpool/ROOT/bename. I couldn't find any place where the entire dataset 
"rpool/ROOT/bename" was being checked thus the addition of the lines you 
mentioned above

> be_utils.c:
> 345 to 350 - Minor nit: The function be_valid_be_description() basically only 
> checks the length but in the future there may be other checks it makes. It 
> just 
> seems a bit awkward to have the comment about length at this point in 
> be_append_grub. However it's not a big deal since at some point we'll be 
> getting 
> rid of these be_print_err() lines anyway...
> 

Good point. I simplified it since it is just a debug message anyway.

> libbe.c:
> 807 Function name in the comment block should be beVerifyDescription.
> 

done.

> One last thing, it looks like this work space hasn't been merged with the 
> latest 
> changes in slim_source so there may be some merge "fun"...

I fixed it and updated WR.

Thanks
Tim

> 
> Other than these items it looks fine to me.
> 
> Thanks!
> -evan
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to