* [Patch] libcldc,cldcli: Use humanized error messages @ 2009-08-27 2:06 Pete Zaitcev 2009-08-27 2:26 ` Jeff Garzik 0 siblings, 1 reply; 4+ messages in thread From: Pete Zaitcev @ 2009-08-27 2:06 UTC (permalink / raw) To: Jeff Garzik; +Cc: Project Hail List Signed-off-by: Pete Zaitcev <zaitcev@redhat.com> diff --git a/include/cld_msg.h b/include/cld_msg.h index 01bda16..e4c8f28 100644 --- a/include/cld_msg.h +++ b/include/cld_msg.h @@ -257,5 +257,6 @@ struct cld_msg_event { extern unsigned long long cld_sid2llu(const uint8_t *sid); extern void __cld_rand64(void *p); +extern const char *cld_errstr(enum cle_err_codes ecode); #endif /* __CLD_MSG_H__ */ diff --git a/lib/common.c b/lib/common.c index fac652b..57f8696 100644 --- a/lib/common.c +++ b/lib/common.c @@ -19,3 +19,29 @@ void __cld_rand64(void *p) v[1] = rand(); } +const char *cld_errstr(enum cle_err_codes ecode) +{ + switch (ecode) { + case CLE_OK: return "Success"; + case CLE_SESS_EXISTS: return "Session exists"; + case CLE_SESS_INVAL: return "Invalid session"; + case CLE_DB_ERR: return "Database error"; + case CLE_BAD_PKT: return "Invalid/corrupted packet"; + case CLE_INODE_INVAL: return "Invalid inode number"; + case CLE_NAME_INVAL: return "Invalid file name"; + case CLE_OOM: return "Server out of memory"; + case CLE_FH_INVAL: return "Invalid file handle"; + case CLE_DATA_INVAL: return "Invalid data packet"; + case CLE_LOCK_INVAL: return "Invalid lock"; + case CLE_LOCK_CONFLICT: return "Conflicting lock held"; + case CLE_LOCK_PENDING: return "Lock waiting to be acquired"; + case CLE_MODE_INVAL: return "Operation incompatible with file mode"; + case CLE_INODE_EXISTS: return "File exists"; + case CLE_DIR_NOTEMPTY: return "Directory not empty"; + case CLE_INTERNAL_ERR: return "Internal error"; + case CLE_TIMEOUT: return "Session timed out"; + case CLE_SIG_INVAL: return "Bad HMAC signature"; + default: return "(unknown)"; + } +} + diff --git a/tools/cldcli.c b/tools/cldcli.c index 635bc6e..eb4ebc4 100644 --- a/tools/cldcli.c +++ b/tools/cldcli.c @@ -66,9 +66,11 @@ struct creq { struct cp_fc_info *cfi; }; +enum { CRESP_MSGSZ = 64 }; + struct cresp { enum thread_codes tcode; - char msg[64]; + char msg[CRESP_MSGSZ]; union { size_t file_len; unsigned int n_records; @@ -117,31 +119,10 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state); static const struct argp argp = { options, parse_opt, NULL, doc }; -static const char *names_cle_err[] = { - [CLE_OK] = "CLE_OK", - [CLE_SESS_EXISTS] = "CLE_SESS_EXISTS", - [CLE_SESS_INVAL] = "CLE_SESS_INVAL", - [CLE_DB_ERR] = "CLE_DB_ERR", - [CLE_BAD_PKT] = "CLE_BAD_PKT", - [CLE_INODE_INVAL] = "CLE_INODE_INVAL", - [CLE_NAME_INVAL] = "CLE_NAME_INVAL", - [CLE_OOM] = "CLE_OOM", - [CLE_FH_INVAL] = "CLE_FH_INVAL", - [CLE_DATA_INVAL] = "CLE_DATA_INVAL", - [CLE_LOCK_INVAL] = "CLE_LOCK_INVAL", - [CLE_LOCK_CONFLICT] = "CLE_LOCK_CONFLICT", - [CLE_LOCK_PENDING] = "CLE_LOCK_PENDING", - [CLE_MODE_INVAL] = "CLE_MODE_INVAL", - [CLE_INODE_EXISTS] = "CLE_INODE_EXISTS", - [CLE_DIR_NOTEMPTY] = "CLE_DIR_NOTEMPTY", - [CLE_INTERNAL_ERR] = "CLE_INTERNAL_ERR", - [CLE_TIMEOUT] = "CLE_TIMEOUT", - [CLE_SIG_INVAL] = "CLE_SIG_INVAL", -}; - static void errc_msg(struct cresp *cresp, enum cle_err_codes errc) { - strcpy(cresp->msg, names_cle_err[errc]); + strncpy(cresp->msg, cld_errstr(errc), CRESP_MSGSZ); + cresp->msg[CRESP_MSGSZ-1] = 0; } static void applog(int prio, const char *fmt, ...) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch] libcldc,cldcli: Use humanized error messages 2009-08-27 2:06 [Patch] libcldc,cldcli: Use humanized error messages Pete Zaitcev @ 2009-08-27 2:26 ` Jeff Garzik 2009-08-27 2:50 ` Pete Zaitcev 0 siblings, 1 reply; 4+ messages in thread From: Jeff Garzik @ 2009-08-27 2:26 UTC (permalink / raw) To: Pete Zaitcev; +Cc: Project Hail List [-- Attachment #1: Type: text/plain, Size: 345 bytes --] On 08/26/2009 10:06 PM, Pete Zaitcev wrote: > Signed-off-by: Pete Zaitcev<zaitcev@redhat.com> applied a modified version, see attached... after that short IRC discussion, I think an array with a range check would be nicer than coding it (though the C compiler should make a nice, efficient table from 'switch' statement for us...) Jeff [-- Attachment #2: patch --] [-- Type: text/plain, Size: 3742 bytes --] commit 469aeb041c69c43dda1051e5911c47b5dbc78721 Author: Pete Zaitcev <zaitcev@redhat.com> Date: Wed Aug 26 22:19:02 2009 -0400 libcldc, cldcli: Use humanized error messages Signed-off-by: Pete Zaitcev <zaitcev@redhat.com> [slight change, error messages kept in array -jg] Signed-off-by: Jeff Garzik <jgarzik@redhat.com> diff --git a/include/cld_msg.h b/include/cld_msg.h index 01bda16..e4c8f28 100644 --- a/include/cld_msg.h +++ b/include/cld_msg.h @@ -257,5 +257,6 @@ struct cld_msg_event { extern unsigned long long cld_sid2llu(const uint8_t *sid); extern void __cld_rand64(void *p); +extern const char *cld_errstr(enum cle_err_codes ecode); #endif /* __CLD_MSG_H__ */ diff --git a/lib/common.c b/lib/common.c index fac652b..a4eda54 100644 --- a/lib/common.c +++ b/lib/common.c @@ -4,6 +4,9 @@ #include <cld-private.h> #include "cld_msg.h" +/* duplicated from tools/cldcli.c; put in common header somewhere? */ +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) + unsigned long long cld_sid2llu(const uint8_t *sid) { const uint64_t *v_le = (const uint64_t *) sid; @@ -19,3 +22,34 @@ void __cld_rand64(void *p) v[1] = rand(); } +static const char *cld_errlist[] = +{ + [CLE_OK] = "Success", + [CLE_SESS_EXISTS] = "Session exists", + [CLE_SESS_INVAL] = "Invalid session", + [CLE_DB_ERR] = "Database error", + [CLE_BAD_PKT] = "Invalid/corrupted packet", + [CLE_INODE_INVAL] = "Invalid inode number", + [CLE_NAME_INVAL] = "Invalid file name", + [CLE_OOM] = "Server out of memory", + [CLE_FH_INVAL] = "Invalid file handle", + [CLE_DATA_INVAL] = "Invalid data packet", + [CLE_LOCK_INVAL] = "Invalid lock", + [CLE_LOCK_CONFLICT] = "Conflicting lock held", + [CLE_LOCK_PENDING] = "Lock waiting to be acquired", + [CLE_MODE_INVAL] = "Operation incompatible with file mode", + [CLE_INODE_EXISTS] = "File exists", + [CLE_DIR_NOTEMPTY] = "Directory not empty", + [CLE_INTERNAL_ERR] = "Internal error", + [CLE_TIMEOUT] = "Session timed out", + [CLE_SIG_INVAL] = "Bad HMAC signature", +}; + +const char *cld_errstr(enum cle_err_codes ecode) +{ + if (ecode >= ARRAY_SIZE(cld_errlist)) + return "(unknown)"; + + return cld_errlist[ecode]; +} + diff --git a/tools/cldcli.c b/tools/cldcli.c index 635bc6e..eb4ebc4 100644 --- a/tools/cldcli.c +++ b/tools/cldcli.c @@ -66,9 +66,11 @@ struct creq { struct cp_fc_info *cfi; }; +enum { CRESP_MSGSZ = 64 }; + struct cresp { enum thread_codes tcode; - char msg[64]; + char msg[CRESP_MSGSZ]; union { size_t file_len; unsigned int n_records; @@ -117,31 +119,10 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state); static const struct argp argp = { options, parse_opt, NULL, doc }; -static const char *names_cle_err[] = { - [CLE_OK] = "CLE_OK", - [CLE_SESS_EXISTS] = "CLE_SESS_EXISTS", - [CLE_SESS_INVAL] = "CLE_SESS_INVAL", - [CLE_DB_ERR] = "CLE_DB_ERR", - [CLE_BAD_PKT] = "CLE_BAD_PKT", - [CLE_INODE_INVAL] = "CLE_INODE_INVAL", - [CLE_NAME_INVAL] = "CLE_NAME_INVAL", - [CLE_OOM] = "CLE_OOM", - [CLE_FH_INVAL] = "CLE_FH_INVAL", - [CLE_DATA_INVAL] = "CLE_DATA_INVAL", - [CLE_LOCK_INVAL] = "CLE_LOCK_INVAL", - [CLE_LOCK_CONFLICT] = "CLE_LOCK_CONFLICT", - [CLE_LOCK_PENDING] = "CLE_LOCK_PENDING", - [CLE_MODE_INVAL] = "CLE_MODE_INVAL", - [CLE_INODE_EXISTS] = "CLE_INODE_EXISTS", - [CLE_DIR_NOTEMPTY] = "CLE_DIR_NOTEMPTY", - [CLE_INTERNAL_ERR] = "CLE_INTERNAL_ERR", - [CLE_TIMEOUT] = "CLE_TIMEOUT", - [CLE_SIG_INVAL] = "CLE_SIG_INVAL", -}; - static void errc_msg(struct cresp *cresp, enum cle_err_codes errc) { - strcpy(cresp->msg, names_cle_err[errc]); + strncpy(cresp->msg, cld_errstr(errc), CRESP_MSGSZ); + cresp->msg[CRESP_MSGSZ-1] = 0; } static void applog(int prio, const char *fmt, ...) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch] libcldc,cldcli: Use humanized error messages 2009-08-27 2:26 ` Jeff Garzik @ 2009-08-27 2:50 ` Pete Zaitcev 2009-08-27 3:04 ` Jeff Garzik 0 siblings, 1 reply; 4+ messages in thread From: Pete Zaitcev @ 2009-08-27 2:50 UTC (permalink / raw) To: Jeff Garzik; +Cc: Project Hail List On Wed, 26 Aug 2009 22:26:50 -0400, Jeff Garzik <jeff@garzik.org> wrote: > On 08/26/2009 10:06 PM, Pete Zaitcev wrote: > > Signed-off-by: Pete Zaitcev<zaitcev@redhat.com> > > applied a modified version, see attached... after that short IRC > discussion, I think an array with a range check would be nicer than > coding it (though the C compiler should make a nice, efficient table > from 'switch' statement for us...) I don't think it's enough. Look: +static const char *cld_errlist[] = +{ + [CLE_OK] = "Success", + [CLE_SESS_EXISTS] = "Session exists", When you do indexed initializations, it's possible to create empty array elements (currently there are none because indexes are contiguous). So, the following needs a NULL check: + return cld_errlist[ecode]; +} -- Pete ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch] libcldc,cldcli: Use humanized error messages 2009-08-27 2:50 ` Pete Zaitcev @ 2009-08-27 3:04 ` Jeff Garzik 0 siblings, 0 replies; 4+ messages in thread From: Jeff Garzik @ 2009-08-27 3:04 UTC (permalink / raw) To: Pete Zaitcev; +Cc: Project Hail List On 08/26/2009 10:50 PM, Pete Zaitcev wrote: > On Wed, 26 Aug 2009 22:26:50 -0400, Jeff Garzik<jeff@garzik.org> wrote: >> On 08/26/2009 10:06 PM, Pete Zaitcev wrote: > >>> Signed-off-by: Pete Zaitcev<zaitcev@redhat.com> >> >> applied a modified version, see attached... after that short IRC >> discussion, I think an array with a range check would be nicer than >> coding it (though the C compiler should make a nice, efficient table >> from 'switch' statement for us...) > > I don't think it's enough. Look: > > +static const char *cld_errlist[] = > +{ > + [CLE_OK] = "Success", > + [CLE_SESS_EXISTS] = "Session exists", > > When you do indexed initializations, it's possible to create > empty array elements (currently there are none because indexes > are contiguous). So, the following needs a NULL check: > > + return cld_errlist[ecode]; > +} True in general, but do you actually see a hole? It seems to me that our error list will always be sequential, without holes. Even if we obsolete an error, we still need to keep it around in the headers and cld_errlist[]. And new error codes will go on the end of the list... Jeff ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-27 3:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-27 2:06 [Patch] libcldc,cldcli: Use humanized error messages Pete Zaitcev 2009-08-27 2:26 ` Jeff Garzik 2009-08-27 2:50 ` Pete Zaitcev 2009-08-27 3:04 ` Jeff Garzik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.