[email protected] 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
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
