johansen at sun.com wrote:
> Evan,
> This looks a lot better. I just have nits at this point.
>
>> - This project will provide for the ability to return a an nvlist of
>> information describing a failure and it's context from calls into libbe.
>
> A nvlist is a pretty complicated way of returning this information.
> Although it is extensible, it probably wouldn't be my first choice for
> storing information this way. Can you explain why you settled on this
> option instead of other possible choices?
The main reason was for the extensibility the nvlist gives us. While there are
easier ways to store this information this seemed like the best way to deal
with
the possibility that we may need to add things in the future. This is
especially
true if we find there are other things needed was the design for the error
handling and logging work for CUD gets going.
However, if there is something that makes more sense here and still gives us
the
extensibility we're looking for, I'm open to any suggestions!
>
>> Structure definitions:
>>
>> internal to the library:
>> struct err_info {
>> union {
>> int ei_err_num; /* this is a be_errno */
>> int ei_op_num; /* enum of libbe operations
>> */
>> int ei_fixit_str_num; /* enum of fixit
>> strings
>> * or URL's */
>> int ei_failed_at; /* enum of function calls
>> */
>> char ei_failed_str[MAXLEN]; /* error string
>> returned
>> * from failure
>> */
>> } ei_info;
>> int ei_err_type; /* The type of failure */
>> };
>>
>
> I would move ei_err_type to the beginning of struct err_info. This way
> offset 0 of the structure always contains the type information. If you
> later change what goes in err_info, or create multiple types of
> err_info, it will be hard to figure out the type if you don't know it
> ahead of time, because ei_err_type could be at a different offset in
> every structure.
Good point I'll move this to the top of the structure.
>
> Thanks for taking the time to incorporate feedback and come up with a
> new revision.
>
> -j
Thanks again for the feedback!
-evan
