Re: [Patch] libcldc,cldcli: Use humanized error messages

[Jeff Garzik <jeff@garzik.org>]

[-- 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, ...)