| Message ID | 1346663926-20188-4-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 09/03/2012 03:18 AM, Wenchao Xia wrote: > This patch contains error handling APIs. > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > libqblock/libqblock-error.c | 44 +++++++++++++++++++++++++++++++++++++++++++ > libqblock/libqblock-error.h | 34 +++++++++++++++++++++++++++++++++ > 2 files changed, 78 insertions(+), 0 deletions(-) > create mode 100644 libqblock/libqblock-error.c > create mode 100644 libqblock/libqblock-error.h > > diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c > new file mode 100644 > index 0000000..28d1d77 > --- /dev/null > +++ b/libqblock/libqblock-error.c > @@ -0,0 +1,44 @@ > +#include "libqblock-error.h" No copyright. Shame. > +++ b/libqblock/libqblock-error.h > @@ -0,0 +1,34 @@ > +#ifndef LIBQBLOCK_ERROR No copyright. Shame. > +#define LIBQBLOCK_ERROR > + > +#include "libqblock-types.h" > + > +#define QB_ERR_MEM_ERR (-1) > +#define QB_ERR_INTERNAL_ERR (-2) > +#define QB_ERR_INVALID_PARAM (-3) > +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100) Would an enum make more sense than #defines? > + > +/* error handling */ > +/** > + * qb_error_get_human_str: get human readable erro string. s/erro/error/ > + * > + * return a human readable string. > + * > + * @broker: operation broker, must be valid. > + * @buf: buf to receive the string. > + * @buf_size: the size of the string buf. > + */ > +void qb_error_get_human_str(struct QBroker *broker, > + char *buf, int buf_size); What happens if buf_size is too small to receive the entire error message? Should this function return int, with negative value on input error?
δΊ 2012-9-3 22:22, Eric Blake ει: > On 09/03/2012 03:18 AM, Wenchao Xia wrote: >> This patch contains error handling APIs. >> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> libqblock/libqblock-error.c | 44 +++++++++++++++++++++++++++++++++++++++++++ >> libqblock/libqblock-error.h | 34 +++++++++++++++++++++++++++++++++ >> 2 files changed, 78 insertions(+), 0 deletions(-) >> create mode 100644 libqblock/libqblock-error.c >> create mode 100644 libqblock/libqblock-error.h >> >> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c >> new file mode 100644 >> index 0000000..28d1d77 >> --- /dev/null >> +++ b/libqblock/libqblock-error.c >> @@ -0,0 +1,44 @@ >> +#include "libqblock-error.h" > > No copyright. Shame. > >> +++ b/libqblock/libqblock-error.h >> @@ -0,0 +1,34 @@ >> +#ifndef LIBQBLOCK_ERROR > > No copyright. Shame. > >> +#define LIBQBLOCK_ERROR >> + >> +#include "libqblock-types.h" >> + >> +#define QB_ERR_MEM_ERR (-1) >> +#define QB_ERR_INTERNAL_ERR (-2) >> +#define QB_ERR_INVALID_PARAM (-3) >> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100) > > Would an enum make more sense than #defines? > >> + >> +/* error handling */ >> +/** >> + * qb_error_get_human_str: get human readable erro string. > > s/erro/error/ > >> + * >> + * return a human readable string. >> + * >> + * @broker: operation broker, must be valid. >> + * @buf: buf to receive the string. >> + * @buf_size: the size of the string buf. >> + */ >> +void qb_error_get_human_str(struct QBroker *broker, >> + char *buf, int buf_size); > > What happens if buf_size is too small to receive the entire error > message? Should this function return int, with negative value on input > error? > if so, the string is truncated. Give negative value resulting an error check for an error check, I guess user would not like it, so provide error check functions which never fail.
about the error number defines, I think using union instead of macro will cause additional trouble: int64_t qb_read() In this case return is type int64_t, and it may return the error number, so using unions for error number would not provide much help. Other issues you mentioned have been fixed in next version. > On 09/03/2012 03:18 AM, Wenchao Xia wrote: >> This patch contains error handling APIs. >> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> libqblock/libqblock-error.c | 44 +++++++++++++++++++++++++++++++++++++++++++ >> libqblock/libqblock-error.h | 34 +++++++++++++++++++++++++++++++++ >> 2 files changed, 78 insertions(+), 0 deletions(-) >> create mode 100644 libqblock/libqblock-error.c >> create mode 100644 libqblock/libqblock-error.h >> >> diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c >> new file mode 100644 >> index 0000000..28d1d77 >> --- /dev/null >> +++ b/libqblock/libqblock-error.c >> @@ -0,0 +1,44 @@ >> +#include "libqblock-error.h" > > No copyright. Shame. > >> +++ b/libqblock/libqblock-error.h >> @@ -0,0 +1,34 @@ >> +#ifndef LIBQBLOCK_ERROR > > No copyright. Shame. > >> +#define LIBQBLOCK_ERROR >> + >> +#include "libqblock-types.h" >> + >> +#define QB_ERR_MEM_ERR (-1) >> +#define QB_ERR_INTERNAL_ERR (-2) >> +#define QB_ERR_INVALID_PARAM (-3) >> +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100) > > Would an enum make more sense than #defines? > >> + >> +/* error handling */ >> +/** >> + * qb_error_get_human_str: get human readable erro string. > > s/erro/error/ > >> + * >> + * return a human readable string. >> + * >> + * @broker: operation broker, must be valid. >> + * @buf: buf to receive the string. >> + * @buf_size: the size of the string buf. >> + */ >> +void qb_error_get_human_str(struct QBroker *broker, >> + char *buf, int buf_size); > > What happens if buf_size is too small to receive the entire error > message? Should this function return int, with negative value on input > error? >
diff --git a/libqblock/libqblock-error.c b/libqblock/libqblock-error.c new file mode 100644 index 0000000..28d1d77 --- /dev/null +++ b/libqblock/libqblock-error.c @@ -0,0 +1,44 @@ +#include "libqblock-error.h" +#include "libqblock-helper.h" + +void qb_error_get_human_str(struct QBroker *broker, + char *buf, int buf_size) +{ + const char *err_ret_str; + switch (broker->err_ret) { + case QB_ERR_MEM_ERR: + err_ret_str = "Not enough memory."; + break; + case QB_ERR_INTERNAL_ERR: + err_ret_str = "Internal error."; + break; + case QB_ERR_INVALID_PARAM: + err_ret_str = "Invalid param."; + break; + default: + err_ret_str = "Unknow error."; + break; + } + if (broker == NULL) { + snprintf(buf, buf_size, "%s", err_ret_str); + return; + } + + if (broker->err_ret == QB_ERR_INTERNAL_ERR) { + snprintf(buf, buf_size, "%s %s errno [%d]. strerror [%s].", + err_ret_str, broker->err_msg, + broker->err_no, strerror(-broker->err_no)); + } else { + snprintf(buf, buf_size, "%s %s", + err_ret_str, broker->err_msg); + } + return; +} + +int qb_error_get_errno(struct QBroker *broker) +{ + if (broker->err_ret == QB_ERR_INTERNAL_ERR) { + return broker->err_no; + } + return 0; +} diff --git a/libqblock/libqblock-error.h b/libqblock/libqblock-error.h new file mode 100644 index 0000000..b11df7c --- /dev/null +++ b/libqblock/libqblock-error.h @@ -0,0 +1,34 @@ +#ifndef LIBQBLOCK_ERROR +#define LIBQBLOCK_ERROR + +#include "libqblock-types.h" + +#define QB_ERR_MEM_ERR (-1) +#define QB_ERR_INTERNAL_ERR (-2) +#define QB_ERR_INVALID_PARAM (-3) +#define QB_ERR_BLOCK_OUT_OF_RANGE (-100) + +/* error handling */ +/** + * qb_error_get_human_str: get human readable erro string. + * + * return a human readable string. + * + * @broker: operation broker, must be valid. + * @buf: buf to receive the string. + * @buf_size: the size of the string buf. + */ +void qb_error_get_human_str(struct QBroker *broker, + char *buf, int buf_size); + +/** + * qb_error_get_errno: get error number, only valid when err_ret is + * QB_ERR_INTERNAL_ERR. + * + * return negative errno or 0 if last error is not QB_ERR_INTERNAL_ERR. + * + * @broker: operation broker. + */ +int qb_error_get_errno(struct QBroker *broker); + +#endif
This patch contains error handling APIs. Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- libqblock/libqblock-error.c | 44 +++++++++++++++++++++++++++++++++++++++++++ libqblock/libqblock-error.h | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 0 deletions(-) create mode 100644 libqblock/libqblock-error.c create mode 100644 libqblock/libqblock-error.h