[stgt] [PATCH 4/4] using concat_printf() in iscsi code
Alexander Nezhinsky
alexandern at mellanox.com
Sat Nov 26 12:28:41 CET 2011
Using struct concat_buf and its associated concat_printf() variant
of snprintf(), replacing buffer_check() and _buffer_check() macros.
This spares explicit checkguards against buffer overflow, and by
explicit passing of concat_buf as the function argument allows
nested calls to functions which append more lines to the buffer.
Signed-off-by: Alexander Nezhinsky <alexandern at mellanox.com>
---
usr/iscsi/iscsid.h | 12 +----
usr/iscsi/isns.c | 22 ++++-----
usr/iscsi/target.c | 127 ++++++++++++++++++---------------------------------
3 files changed, 56 insertions(+), 105 deletions(-)
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c17ed13..e8d625b 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -28,6 +28,7 @@
#include "param.h"
#include "log.h"
#include "tgtd.h"
+#include "util.h"
#include "iscsi_proto.h"
#include "iscsi_if.h"
@@ -351,19 +352,10 @@ extern void iscsi_exit(void);
/* isns.c */
extern int isns_init(void);
extern void isns_exit(void);
-extern int isns_show(char *buf, int rest);
+extern int isns_show(struct concat_buf *b);
extern int isns_update(char *params);
extern int isns_scn_access(int tid, char *name);
extern int isns_target_register(char *name);
extern int isns_target_deregister(char *name);
-#define buffer_check(buf, total, len, rest) \
-({ \
- buf += len; \
- total += len; \
- rest -= len; \
- if (!rest) \
- break; \
-})
-
#endif /* ISCSID_H */
diff --git a/usr/iscsi/isns.c b/usr/iscsi/isns.c
index 0afd251..fbe73ac 100644
--- a/usr/iscsi/isns.c
+++ b/usr/iscsi/isns.c
@@ -1068,20 +1068,16 @@ void isns_exit(void)
free(rxbuf);
}
-int isns_show(char *buf, int rest)
+int isns_show(struct concat_buf *b)
{
- int total = 0, max = rest;
-
- shprintf(total, buf, rest, "iSNS:\n");
- shprintf(total, buf, rest, _TAB1 "iSNS=%s\n",
- use_isns ? "On" : "Off");
- shprintf(total, buf, rest, _TAB1 "iSNSServerIP=%s\n", isns_addr);
- shprintf(total, buf, rest, _TAB1 "iSNSServerPort=%d\n", isns_port);
- shprintf(total, buf, rest, _TAB1 "iSNSAccessControl=%s\n",
- use_isns_ac ? "On" : "Off");
- return total;
-overflow:
- return max;
+ concat_printf(b, "iSNS:\n");
+ concat_printf(b, _TAB1 "iSNS=%s\n", use_isns ? "On" : "Off");
+ concat_printf(b, _TAB1 "iSNSServerIP=%s\n", isns_addr);
+ concat_printf(b, _TAB1 "iSNSServerPort=%d\n", isns_port);
+ concat_printf(b, _TAB1 "iSNSAccessControl=%s\n",
+ use_isns_ac ? "On" : "Off");
+
+ return concat_buf_ret(b);
}
enum {
diff --git a/usr/iscsi/target.c b/usr/iscsi/target.c
index 9546035..ab8f11b 100644
--- a/usr/iscsi/target.c
+++ b/usr/iscsi/target.c
@@ -36,6 +36,7 @@
#include "tgtadm.h"
#include "tgtd.h"
#include "target.h"
+#include "util.h"
LIST_HEAD(iscsi_targets_list);
@@ -562,30 +563,20 @@ int iscsi_target_update(int mode, int op, int tid, uint64_t sid, uint64_t lun,
return err;
}
-static int show_iscsi_param(char *buf, struct param *param, int rest)
+static int show_iscsi_param(struct param *param, struct concat_buf *b)
{
- int i, len, total;
- char value[64];
struct iscsi_key *keys = session_keys;
+ int i;
+ char value[64];
- for (i = total = 0; session_keys[i].name; i++) {
+ for (i = 0; session_keys[i].name; i++) {
param_val_to_str(keys, i, param[i].val, value);
- len = snprintf(buf, rest, "%s=%s\n", keys[i].name, value);
- buffer_check(buf, total, len, rest);
+ concat_printf(b, "%s=%s\n", keys[i].name, value);
}
- return total;
+ return concat_buf_ret(b);
}
-#define __buffer_check(buf, total, len, rest) \
-({ \
- buf += len; \
- total += len; \
- rest -= len; \
- if (!rest) \
- return total; \
-})
-
static struct iscsi_session *iscsi_target_find_session(
struct iscsi_target *target,
uint64_t sid)
@@ -602,26 +593,21 @@ static struct iscsi_session *iscsi_target_find_session(
}
static int iscsi_target_show_session(struct iscsi_target *target, uint64_t sid,
- char *buf, int rest)
+ struct concat_buf *b)
{
- int len = 0, total = 0;
struct iscsi_session *session;
session = iscsi_target_find_session(target, sid);
- if (session) {
- len = show_iscsi_param(buf, session->session_param, rest);
- __buffer_check(buf, total, len, rest);
-
- }
+ if (session)
+ show_iscsi_param(session->session_param, b);
- return total;
+ return concat_buf_ret(b);
}
static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
- char *buf, int rest)
+ struct concat_buf *b)
{
- int len = 0, total = 0;
struct iscsi_session *session;
struct iscsi_connection *conn;
@@ -629,8 +615,7 @@ static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
if (session) {
list_for_each_entry(conn, &session->conn_list, clist) {
- len = snprintf(buf, rest,
- "rxdata_octets: %" PRIu64 "\n"
+ concat_printf(b, "rxdata_octets: %" PRIu64 "\n"
"txdata_octets: %" PRIu64 "\n"
"dataout_pdus: %d\n"
"datain_pdus: %d\n"
@@ -642,30 +627,27 @@ static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
conn->stats.datain_pdus,
conn->stats.scsicmd_pdus,
conn->stats.scsirsp_pdus);
- __buffer_check(buf, total, len, rest);
}
}
- return total;
+ return concat_buf_ret(b);
}
static int iscsi_target_show_connections(struct iscsi_target *target,
uint64_t sid,
- char *buf, int rest)
+ struct concat_buf *b)
{
struct iscsi_session *session;
struct iscsi_connection *conn;
char addr[128];
- int len = 0, total = 0;
list_for_each_entry(session, &target->sessions_list, slist) {
list_for_each_entry(conn, &session->conn_list, clist) {
memset(addr, 0, sizeof(addr));
conn->tp->ep_show(conn, addr, sizeof(addr));
-
- len = snprintf(buf, rest, "Session: %u\n"
+ concat_printf(b, "Session: %u\n"
_TAB1 "Connection: %u\n"
_TAB2 "Initiator: %s\n"
_TAB2 "%s\n",
@@ -673,16 +655,14 @@ static int iscsi_target_show_connections(struct iscsi_target *target,
conn->cid,
session->initiator,
addr);
- __buffer_check(buf, total, len, rest);
}
}
- return total;
+ return concat_buf_ret(b);
}
static int iscsi_target_show_portals(struct iscsi_target *target, uint64_t sid,
- char *buf, int rest)
+ struct concat_buf *b)
{
- int len = 0, total = 0;
struct iscsi_portal *portal;
list_for_each_entry(portal, &iscsi_portals_list,
@@ -690,57 +670,43 @@ static int iscsi_target_show_portals(struct iscsi_target *target, uint64_t sid,
int is_ipv6;
is_ipv6 = strchr(portal->addr, ':') != NULL;
- len = snprintf(buf, rest,
- "Portal: %s%s%s:%d,%d\n",
+ concat_printf(b, "Portal: %s%s%s:%d,%d\n",
is_ipv6 ? "[" : "",
portal->addr,
is_ipv6 ? "]" : "",
portal->port ? portal->port : ISCSI_LISTEN_PORT,
portal->tpgt);
- __buffer_check(buf, total, len, rest);
}
- return total;
-
+ return concat_buf_ret(b);
}
-static int show_redirect_info(struct iscsi_target* target, char *buf, int rest)
+static int show_redirect_info(struct iscsi_target* target, struct concat_buf *b)
{
- int len, total = 0;
-
- len = snprintf(buf, rest, "RedirectAddress=%s\n", target->redirect_info.addr);
- __buffer_check(buf, total, len, rest);
- len = snprintf(buf, rest, "RedirectPort=%s\n", target->redirect_info.port);
- __buffer_check(buf, total, len, rest);
- if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_TEMP) {
- len = snprintf(buf, rest, "RedirectReason=Temporary\n");
- __buffer_check(buf, total, len, rest);
- } else if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_PERM) {
- len = snprintf(buf, rest, "RedirectReason=Permanent\n");
- __buffer_check(buf, total, len, rest);
- } else {
- len = snprintf(buf, rest, "RedirectReason=Unknown\n");
- __buffer_check(buf, total, len, rest);
- }
+ concat_printf(b, "RedirectAddress=%s\n", target->redirect_info.addr);
+ concat_printf(b, "RedirectPort=%s\n", target->redirect_info.port);
+ if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_TEMP)
+ concat_printf(b, "RedirectReason=Temporary\n");
+ else if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_PERM)
+ concat_printf(b, "RedirectReason=Permanent\n");
+ else
+ concat_printf(b, "RedirectReason=Unknown\n");
- return total;
+ return concat_buf_ret(b);
}
-static int show_redirect_callback(struct iscsi_target *target, char *buf, int rest)
+static int show_redirect_callback(struct iscsi_target *target, struct concat_buf *b)
{
- int len, total = 0;
-
- len = snprintf(buf, rest, "RedirectCallback=%s\n", target->redirect_info.callback);
- __buffer_check(buf, total, len, rest);
+ concat_printf(b, "RedirectCallback=%s\n", target->redirect_info.callback);
- return total;
+ return concat_buf_ret(b);
}
int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lun,
char *buf, int rest)
{
struct iscsi_target* target = NULL;
- int len, total = 0;
+ struct concat_buf b;
if (mode != MODE_SYSTEM && mode != MODE_PORTAL) {
target = target_find_by_id(tid);
@@ -748,38 +714,35 @@ int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lu
return -TGTADM_NO_TARGET;
}
+ concat_buf_init(&b, buf, rest);
+
switch (mode) {
case MODE_SYSTEM:
- total = isns_show(buf, rest);
+ isns_show(&b);
break;
case MODE_TARGET:
if (target->redirect_info.callback)
- len = show_redirect_callback(target, buf, rest);
+ show_redirect_callback(target, &b);
else if (strlen(target->redirect_info.addr))
- len = show_redirect_info(target, buf, rest);
+ show_redirect_info(target, &b);
else
- len = show_iscsi_param(buf, target->session_param, rest);
- total += len;
+ show_iscsi_param(target->session_param, &b);
break;
case MODE_SESSION:
- len = iscsi_target_show_session(target, sid, buf, rest);
- total += len;
+ iscsi_target_show_session(target, sid, &b);
break;
case MODE_PORTAL:
- len = iscsi_target_show_portals(target, sid, buf, rest);
- total += len;
+ iscsi_target_show_portals(target, sid, &b);
break;
case MODE_CONNECTION:
- len = iscsi_target_show_connections(target, sid, buf, rest);
- total += len;
+ iscsi_target_show_connections(target, sid, &b);
break;
case MODE_STATS:
- len = iscsi_target_show_stats(target, sid, buf, rest);
- total += len;
+ iscsi_target_show_stats(target, sid, &b);
break;
default:
break;
}
- return total;
+ return concat_buf_ret(&b);
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the stgt
mailing list