From eaa99a8770f700ebca1206578663108c71300ee6 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 14 Apr 2017 01:03:25 +0000 Subject: [PATCH] Fix a few minor "extra" compiler warnings (mostly "const" issues) Add "XXX" comments in a few places about potential problems seen in SCST code, for future investigation and possible repair. Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7123 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/iscsi.c | 7 ++++++- iscsi-scst/kernel/nthread.c | 2 +- iscsi-scst/usr/chap.c | 5 +++++ iscsi-scst/usr/config.c | 2 +- scst/include/scst.h | 4 ++-- scst/src/dev_handlers/scst_vdisk.c | 9 ++++++--- scst/src/scst_lib.c | 14 +++++++------- scst/src/scst_targ.c | 8 ++++++-- usr/fileio/common.h | 2 +- 9 files changed, 35 insertions(+), 18 deletions(-) diff --git a/iscsi-scst/kernel/iscsi.c b/iscsi-scst/kernel/iscsi.c index d20799446..1a5ac0991 100644 --- a/iscsi-scst/kernel/iscsi.c +++ b/iscsi-scst/kernel/iscsi.c @@ -509,6 +509,11 @@ void cmnd_done(struct iscsi_cmnd *cmnd) if (cmnd->sg != &dummy_sg) scst_free_sg(cmnd->sg, cmnd->sg_cnt); #ifdef CONFIG_SCST_DEBUG + /* + * We can make correct behavior depend on this, so + * a bug can disappear whenever you enable debugging, + * but probability of failure on zero/NULL is much bigger + */ cmnd->own_sg = 0; cmnd->sg = NULL; cmnd->sg_cnt = -1; @@ -2911,7 +2916,7 @@ out: return; } -static void set_cork(struct socket *sock, int on) +static inline void set_cork(struct socket *sock, int on) { int opt = on; mm_segment_t oldfs; diff --git a/iscsi-scst/kernel/nthread.c b/iscsi-scst/kernel/nthread.c index 1da2f808d..b61540f7c 100644 --- a/iscsi-scst/kernel/nthread.c +++ b/iscsi-scst/kernel/nthread.c @@ -1687,7 +1687,7 @@ static int tx_padding(struct iscsi_cmnd *cmnd, int state) int res, rest = cmnd->conn->write_size; struct msghdr msg = {.msg_flags = MSG_NOSIGNAL | MSG_DONTWAIT}; struct kvec iov; - static uint32_t padding; + static const uint32_t padding; BUG_ON(rest < 1); BUG_ON(rest >= sizeof(uint32_t)); diff --git a/iscsi-scst/usr/chap.c b/iscsi-scst/usr/chap.c index dfefae1ac..7b70fbd5c 100644 --- a/iscsi-scst/usr/chap.c +++ b/iscsi-scst/usr/chap.c @@ -105,6 +105,9 @@ static u8 decode_base64_digit(char base64) else if ((base64 >= '0') && (base64 <= '9')) return 52 + (base64 - '0'); else + //XXX This return value should be unsigned; and anyway + //XXX in case of a bad character in the string, our + //XXX caller (sometimes) checks for 65, not 255 or -1 return -1; } } @@ -146,6 +149,8 @@ static void decode_base64_string(char *string, u8 *intnum, int int_len) num[1] = decode_base64_digit(string[count + 1]); num[2] = decode_base64_digit(string[count + 2]); num[3] = decode_base64_digit(string[count + 3]); + //XXX Check for the special "bad character in string" value here like above? + //XXX Also check the string for missing/incorrect padding? if ((num[0] == 64) || (num[1] == 64)) return; if (num[2] == 64) { diff --git a/iscsi-scst/usr/config.c b/iscsi-scst/usr/config.c index ba2ed557e..58469020c 100644 --- a/iscsi-scst/usr/config.c +++ b/iscsi-scst/usr/config.c @@ -1144,7 +1144,7 @@ int config_load(const char *config_name) err = config_isns_load(buf); if ((err == 0) && (isns_server != NULL)) { - int rc = isns_init(); + rc = isns_init(); if (rc != 0) { log_error("iSNS server %s init failed: %d", isns_server, rc); diff --git a/scst/include/scst.h b/scst/include/scst.h index d861d2906..e6c31828a 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -4081,8 +4081,8 @@ static inline enum scst_exec_context __scst_estimate_context(bool atomic) else if (in_atomic()) return SCST_CONTEXT_DIRECT_ATOMIC; else - return atomic ? SCST_CONTEXT_DIRECT : - SCST_CONTEXT_DIRECT_ATOMIC; + return atomic ? SCST_CONTEXT_DIRECT_ATOMIC : + SCST_CONTEXT_DIRECT; #else return SCST_CONTEXT_THREAD; #endif diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index 3bf96a32b..41d93781e 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -83,7 +83,7 @@ static struct scst_trace_log vdisk_local_trace_tbl[] = { #define SCST_FIO_VENDOR "SCST_FIO" #define SCST_BIO_VENDOR "SCST_BIO" /* 4 byte ASCII Product Revision Level - left aligned */ -#define SCST_FIO_REV " 330" +#define SCST_FIO_REV "330 " #define MAX_USN_LEN (20+1) /* For '\0' */ #define MAX_INQ_VEND_SPECIFIC_LEN (INQ_BUF_SZ - 96) @@ -2081,8 +2081,11 @@ static int vdisk_format_dif(struct scst_cmd *cmd, uint64_t start_lba, goto out_set_fs; } else if (err < full_len) { /* - * Probably that's wrong, but sometimes write() returns - * value less, than requested. Let's restart. + * If a write() is interrupted by a signal handler before + * any bytes are written, then the call fails with the + * error EINTR; if it is interrupted after at least one + * byte has been written, the call succeeds, and returns + * the number of bytes written -- manpage write(2) */ left += full_len - err; done -= full_len - err; diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 96764683d..36456f224 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -11695,15 +11695,15 @@ uint64_t scst_unpack_lun(const uint8_t *lun, int len) case 2: break; case 8: - if ((*((__be64 *)lun) & cpu_to_be64(0x0000FFFFFFFFFFFFLL)) != 0) + if ((*((__be64 const *)lun) & cpu_to_be64(0x0000FFFFFFFFFFFFLL)) != 0) goto out_err; break; case 4: - if (*((__be16 *)&lun[2]) != 0) + if (*((__be16 const *)&lun[2]) != 0) goto out_err; break; case 6: - if (*((__be32 *)&lun[2]) != 0) + if (*((__be32 const *)&lun[2]) != 0) goto out_err; break; case 1: @@ -13870,10 +13870,10 @@ EXPORT_SYMBOL(scst_reassign_retained_sess_states); char *scst_get_next_lexem(char **token_str) { char *p, *q; - static const char blank = '\0'; + static char blank = '\0'; if ((token_str == NULL) || (*token_str == NULL)) - return (char *)␣ + return ␣ for (p = *token_str; (*p != '\0') && (isspace(*p) || (*p == '=')); p++) ; @@ -14874,7 +14874,7 @@ int scst_write_file_transactional(const char *name, const char *name1, pos = signature_len+1; - res = vfs_write(file, (void __force __user *)buf, size, &pos); + res = vfs_write(file, (void const __force __user *)buf, size, &pos); if (res != size) goto write_error; @@ -14885,7 +14885,7 @@ int scst_write_file_transactional(const char *name, const char *name1, } pos = 0; - res = vfs_write(file, (void __force __user *)signature, signature_len, &pos); + res = vfs_write(file, (void const __force __user *)signature, signature_len, &pos); if (res != signature_len) goto write_error; diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 29db7baf7..5ed8ead11 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -1552,9 +1552,13 @@ static int scst_prepare_space(struct scst_cmd *cmd) goto alloc; else if (r == 0) { if (unlikely(cmd->bufflen == 0)) { - /* See comment in scst_alloc_space() */ - if (cmd->sg == NULL) + if (cmd->sg == NULL) { + /* + * Let's still have a buffer for uniformity, + * scst_alloc_space() will handle bufflen 0 + */ goto alloc; + } } cmd->tgt_i_data_buf_alloced = 1; diff --git a/usr/fileio/common.h b/usr/fileio/common.h index 7c1bf6239..943da91b7 100644 --- a/usr/fileio/common.h +++ b/usr/fileio/common.h @@ -30,7 +30,7 @@ /* 8 byte ASCII Vendor */ #define VENDOR "SCST_USR" /* 4 byte ASCII Product Revision Level - left aligned */ -#define FIO_REV " 330" +#define FIO_REV "330 " #define MAX_USN_LEN (20+1) /* For '\0' */