[stgt] [PATCH 3/4] using concat_printf() in target.c
Alexander Nezhinsky
alexandern at mellanox.com
Sat Nov 26 12:26:41 CET 2011
Using struct concat_buf and its associated concat_printf() variant
of snprintf(), replacing shprintf() macro.
This spares explicit checkguards against buffer overflow.
When an overflow does occur, it can be either just reported or handled by
reallocation of memory buffer and retrying the content generation.
Currently only two specific cases have been handled by realloc.
Meanwhile they are left as is, only overflow condition check is updated.
It is reasonable to implement buffer realloc as the standard automatic
recovery policy. Using concat_buf allows doing it rather painlessly.
To be addressed in a separate further patch.
Signed-off-by: Alexander Nezhinsky <alexandern at mellanox.com>
---
usr/mgmt.c | 4 +-
usr/target.c | 98 +++++++++++++++++++++++++++------------------------------
2 files changed, 48 insertions(+), 54 deletions(-)
diff --git a/usr/mgmt.c b/usr/mgmt.c
index 26a360d..758d335 100644
--- a/usr/mgmt.c
+++ b/usr/mgmt.c
@@ -140,7 +140,7 @@ static int target_mgmt(int lld_no, struct mgmt_task *mtask)
if (req->tid < 0) {
retry:
err = tgt_target_show_all(mtask->buf, mtask->bsize);
- if (err == mtask->bsize) {
+ if (err == -ENOSPC) {
char *p;
mtask->bsize <<= 1;
p = realloc(mtask->buf, mtask->bsize);
@@ -270,7 +270,7 @@ static int account_mgmt(int lld_no, struct mgmt_task *mtask)
case OP_SHOW:
retry:
err = account_show(mtask->buf, mtask->bsize);
- if (err == mtask->bsize) {
+ if (err == -ENOSPC) {
char *p;
mtask->bsize <<= 1;
p = realloc(mtask->buf, mtask->bsize);
diff --git a/usr/target.c b/usr/target.c
index dd4b358..4570aa8 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -1760,17 +1760,18 @@ static char *print_type(int type)
int tgt_target_show_all(char *buf, int rest)
{
- int total = 0, max = rest;
char strflags[128];
struct target *target;
struct scsi_lu *lu;
struct acl_entry *acl;
struct iqn_acl_entry *iqn_acl;
struct it_nexus *nexus;
+ struct concat_buf b;
+
+ concat_buf_init(&b, buf, rest);
list_for_each_entry(target, &target_list, target_siblings) {
- shprintf(total, buf, rest,
- "Target %d: %s\n"
+ concat_printf(&b, "Target %d: %s\n"
_TAB1 "System information:\n"
_TAB2 "Driver: %s\n"
_TAB2 "State: %s\n",
@@ -1779,18 +1780,18 @@ int tgt_target_show_all(char *buf, int rest)
tgt_drivers[target->lid]->name,
target_state_name(target->target_state));
- shprintf(total, buf, rest, _TAB1 "I_T nexus information:\n");
+ concat_printf(&b, _TAB1 "I_T nexus information:\n");
list_for_each_entry(nexus, &target->it_nexus_list, nexus_siblings) {
- shprintf(total, buf, rest, _TAB2 "I_T nexus: %" PRIu64 "\n",
- nexus->itn_id);
+ concat_printf(&b, _TAB2 "I_T nexus: %" PRIu64 "\n",
+ nexus->itn_id);
if (nexus->info)
- shprintf(total, buf, rest, "%s", nexus->info);
+ concat_printf(&b, "%s", nexus->info);
}
- shprintf(total, buf, rest, _TAB1 "LUN information:\n");
+ concat_printf(&b, _TAB1 "LUN information:\n");
list_for_each_entry(lu, &target->device_list, device_siblings)
- shprintf(total, buf, rest,
+ concat_printf(&b,
_TAB2 "LUN: %" PRIu64 "\n"
_TAB3 "Type: %s\n"
_TAB3 "SCSI ID: %s\n"
@@ -1821,32 +1822,29 @@ int tgt_target_show_all(char *buf, int rest)
!strcmp(tgt_drivers[target->lid]->name, "iser")) {
int i, aid;
- shprintf(total, buf, rest, _TAB1
- "Account information:\n");
+ concat_printf(&b, _TAB1 "Account information:\n");
for (i = 0; i < target->account.nr_inaccount; i++) {
aid = target->account.in_aids[i];
- shprintf(total, buf, rest, _TAB2 "%s\n",
- __account_lookup_id(aid)->user);
+ concat_printf(&b, _TAB2 "%s\n",
+ __account_lookup_id(aid)->user);
}
if (target->account.out_aid) {
aid = target->account.out_aid;
- shprintf(total, buf, rest,
- _TAB2 "%s (outgoing)\n",
+ concat_printf(&b, _TAB2 "%s (outgoing)\n",
__account_lookup_id(aid)->user);
}
}
- shprintf(total, buf, rest, _TAB1 "ACL information:\n");
+ concat_printf(&b, _TAB1 "ACL information:\n");
list_for_each_entry(acl, &target->acl_list, aclent_list)
- shprintf(total, buf, rest, _TAB2 "%s\n", acl->address);
+ concat_printf(&b, _TAB2 "%s\n", acl->address);
list_for_each_entry(iqn_acl, &target->iqn_acl_list, iqn_aclent_list)
- shprintf(total, buf, rest, _TAB2 "%s\n", iqn_acl->name);
+ concat_printf(&b, _TAB2 "%s\n", iqn_acl->name);
}
- return total;
-overflow:
- return max;
+
+ return concat_buf_ret(&b);
}
char *tgt_targetname(int tid)
@@ -2050,18 +2048,18 @@ int tgt_portal_destroy(int lld, char *args)
int account_show(char *buf, int rest)
{
- int total = 0, max = rest;
struct account_entry *ac;
+ struct concat_buf b;
+
+ concat_buf_init(&b, buf, rest);
if (!list_empty(&account_list))
- shprintf(total, buf, rest, "Account list:\n");
+ concat_printf(&b, "Account list:\n");
list_for_each_entry(ac, &account_list, account_siblings)
- shprintf(total, buf, rest, _TAB1 "%s\n", ac->user);
+ concat_printf(&b, _TAB1 "%s\n", ac->user);
- return total;
-overflow:
- return max;
+ return concat_buf_ret(&b);
}
static struct {
@@ -2104,9 +2102,9 @@ int system_set_state(char *str)
int system_show(int mode, char *buf, int rest)
{
- int total = 0, max = rest;
struct backingstore_template *bst;
struct device_type_template *devt;
+ struct concat_buf b;
int i;
char strflags[128];
@@ -2114,51 +2112,47 @@ int system_show(int mode, char *buf, int rest)
if (mode != MODE_SYSTEM)
return 0;
- shprintf(total, buf, rest, "System:\n");
- shprintf(total, buf, rest, _TAB1 "State: %s\n",
- system_state_name(sys_state));
+ concat_buf_init(&b, buf, rest);
+
+ concat_printf(&b, "System:\n");
+ concat_printf(&b, _TAB1 "State: %s\n", system_state_name(sys_state));
+ concat_printf(&b, _TAB1 "debug: %s\n", is_debug ? "on" : "off");
- shprintf(total, buf, rest, "LLDs:\n");
+ concat_printf(&b, "LLDs:\n");
for (i = 0; tgt_drivers[i]; i++) {
- shprintf(total, buf, rest, _TAB1 "%s: %s\n",
- tgt_drivers[i]->name,
- driver_state_name(tgt_drivers[i]));
+ concat_printf(&b, _TAB1 "%s: %s\n", tgt_drivers[i]->name,
+ driver_state_name(tgt_drivers[i]));
}
- shprintf(total, buf, rest, "Backing stores:\n");
+ concat_printf(&b, "Backing stores:\n");
list_for_each_entry(bst, &bst_list, backingstore_siblings) {
if (!bst->bs_oflags_supported)
- shprintf(total, buf, rest, _TAB1 "%s\n", bst->bs_name);
+ concat_printf(&b, _TAB1 "%s\n", bst->bs_name);
else
- shprintf(total, buf, rest, _TAB1 "%s (bsoflags %s)\n",
- bst->bs_name,
- open_flags_to_str(strflags, bst->bs_oflags_supported));
+ concat_printf(&b, _TAB1 "%s (bsoflags %s)\n", bst->bs_name,
+ open_flags_to_str(strflags, bst->bs_oflags_supported));
}
- shprintf(total, buf, rest, "Device types:\n");
+ concat_printf(&b, "Device types:\n");
list_for_each_entry(devt, &device_type_list, device_type_siblings)
- shprintf(total, buf, rest, _TAB1 "%s\n", print_type(devt->type));
+ concat_printf(&b, _TAB1 "%s\n", print_type(devt->type));
if (global_target.account.nr_inaccount) {
int i, aid;
- shprintf(total, buf, rest,
- "Account information:\n");
+ concat_printf(&b, _TAB1 "%s\n", "Account information:\n");
for (i = 0; i < global_target.account.nr_inaccount; i++) {
aid = global_target.account.in_aids[i];
- shprintf(total, buf, rest, _TAB1 "%s\n",
- __account_lookup_id(aid)->user);
+ concat_printf(&b, _TAB1 "%s\n",
+ __account_lookup_id(aid)->user);
}
if (global_target.account.out_aid) {
aid = global_target.account.out_aid;
- shprintf(total, buf, rest,
- _TAB1 "%s (outgoing)\n",
- __account_lookup_id(aid)->user);
+ concat_printf(&b, _TAB1 "%s (outgoing)\n",
+ __account_lookup_id(aid)->user);
}
}
- return total;
-overflow:
- return max;
+ return concat_buf_ret(&b);
}
int is_system_available(void)
--
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