Jean McCormack wrote:
> be_activate.c
> line 606+
> I'm concerned that err is declared a uint_t and then is returned. Note
> that the function returns an int.
> I also wonder about the code (you didn't modify it but deals with err
> and errno) at lines 634-638.
>
I'm not sure how or why this ended up being unit_t but it should be an int.
As for lines 634-638 we're capturing the errno and translating it into a BE
error code.
> be_create.c:
> line 2404: This will be ok in this case but note that before i was
> undefined and now you've initialized it to 0.
>
> be_mount.c:
> line 680: should that be comparing to != BE_SUCCESS?
>
> be_rename.c:
> line 139: I'm wondering how ret can be anything but 0 at this point? Is
> the else dead code?
ret should be getting set at line 134 but it looks like that got removed at
some
point. :-(
it should look more like this:
134 if ((ret = zpool_iter(g_zfs, be_find_zpool_callback, &bt)) == 0) {
135 be_print_err(gettext("be_rename: failed to "
136 "find zpool for BE (%s)\n"), bt.obe_name);
137 be_zfs_fini();
138 return (BE_ERR_BE_NOENT);
139 } else if (ret < 0) {
140 be_print_err(gettext("be_rename: zpool_iter failed:
%s\n"),
141 libzfs_error_description(g_zfs));
142 ret = zfs_err_to_be_err(g_zfs);
143 be_zfs_fini();
144 return (ret);
145 }
>
> be_utils.c
> line 1114: Should you be comparing to BE_SUCCESS?
> line 3296: same thing
>
> be_zones.c:
> line 412: should you be comparing to BE_SUCCESS?
>
> You might want to look for the general case where you see something like
> this:
>
> ret = be_function_call() != 0
>
> You might need to be comparing to BE_SUCCESS
>
> Jean
>
> Keith Mitchell wrote:
>> Hello,
>>
>> I'd like a CR for my fix for 3837. See webrev below.
>>
>> Note that I have not yet run tests, but, due to the nature of the
>> changes, I think it's unlikely tests these changes will cause
>> problems. I *will* run tests soon, and if for some reason something
>> fails, will fix and re-post.
>>
>> http://cr.opensolaris.org/~kemitche/3837/
>>
>> Thanks,
>> Keith
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
