Looks Good to me. Ethan
On Wed, Mar 9, 2011 at 12:42 PM, Ben Pfaff <[email protected]> wrote: > On Fri, Mar 04, 2011 at 01:36:29PM -0800, Ethan Jackson wrote: >> > +#define OVSDB_BUG(MSG) \ >> > + ovsdb_internal_error(NULL, __FILE__, __LINE__, "%s", MSG) >> > +#define OVSDB_WRAP_BUG(MSG, ERROR) \ >> > + ovsdb_internal_error(ERROR, __FILE__, __LINE__, "%s", MSG) >> >> I think a comment to the effect that OVSDB_WRAP_BUG takes ownership of >> 'ERROR' might be useful. I was surprised at that when reading the >> code. Your call though, it's fine either way. > > That is a good idea. Thank you, I added some comments: > > diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c > index 4a60d4f..3b90b16 100644 > --- a/lib/ovsdb-error.c > +++ b/lib/ovsdb-error.c > @@ -114,6 +114,13 @@ ovsdb_wrap_error(struct ovsdb_error *error, const char > *details, ...) > return error; > } > > +/* Returns an ovsdb_error that represents an internal error for file name > + * 'file' and line number 'line', with 'details' (formatted as with printf()) > + * as the associated message. The caller is responsible for freeing the > + * returned error. > + * > + * If 'inner_error' is nonnull then the returned error is wrapped around > + * 'inner_error'. Takes ownership of 'inner_error'. */ > struct ovsdb_error * > ovsdb_internal_error(struct ovsdb_error *inner_error, > const char *file, int line, const char *details, ...) > diff --git a/lib/ovsdb-error.h b/lib/ovsdb-error.h > index f4a5a69..89b0c19 100644 > --- a/lib/ovsdb-error.h > +++ b/lib/ovsdb-error.h > @@ -40,8 +40,18 @@ struct ovsdb_error *ovsdb_internal_error(struct > ovsdb_error *error, > const char *details, ...) > PRINTF_FORMAT(4, 5) > WARN_UNUSED_RESULT; > + > +/* Returns a pointer to an ovsdb_error that represents an internal error for > + * the current file name and line number with MSG as the associated message. > + * The caller is responsible for freeing the internal error. */ > #define OVSDB_BUG(MSG) \ > ovsdb_internal_error(NULL, __FILE__, __LINE__, "%s", MSG) > + > +/* Returns a pointer to an ovsdb_error that represents an internal error for > + * the current file name and line number, with MSG as the associated message. > + * If ERROR is nonnull then the internal error is wrapped around ERROR. > Takes > + * ownership of ERROR. The caller is responsible for freeing the returned > + * error. */ > #define OVSDB_WRAP_BUG(MSG, ERROR) \ > ovsdb_internal_error(ERROR, __FILE__, __LINE__, "%s", MSG) > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
