Skip to content

Commit

Permalink
Fix problems reported by coverity in NDMP code.
Browse files Browse the repository at this point in the history 
Only fix the obvious problems of code that actually is used.
Change sprintf to snprintf where fixed buffer size is used.
  • Loading branch information
Marco van Wieringen committed Feb 17, 2015
1 parent 677c5c1 commit 839923a
Show file tree
Hide file tree
Showing 17 changed files with 149 additions and 90 deletions.
17 changes: 10 additions & 7 deletions src/ndmp/ndma_comm_dispatch.c
View file
Expand Up @@ -1577,7 +1577,8 @@ ndmp_sxa_data_start_backup (struct ndm_session *sess,
return rc; /* already tattled */
}

strcpy (sess->data_acb->bu_type, request->bu_type);
strncpy (sess->data_acb->bu_type, request->bu_type, sizeof(sess->data_acb->bu_type) - 1);
sess->data_acb->bu_type[sizeof(sess->data_acb->bu_type) - 1] = '\0';

error = data_copy_environment (sess,
request->env.env_val, request->env.env_len);
Expand Down Expand Up @@ -1639,7 +1640,8 @@ ndmp_sxa_data_start_recover (struct ndm_session *sess,
return rc; /* already tattled */
}

strcpy (sess->data_acb->bu_type, request->bu_type);
strncpy (sess->data_acb->bu_type, request->bu_type, sizeof(sess->data_acb->bu_type) - 1);
sess->data_acb->bu_type[sizeof(sess->data_acb->bu_type) - 1] = '\0';

error = data_copy_environment (sess,
request->env.env_val, request->env.env_len);
Expand Down Expand Up @@ -1800,7 +1802,8 @@ ndmp_sxa_data_start_recover_filehist (struct ndm_session *sess,
return rc; /* already tattled */
}

strcpy (sess->data_acb->bu_type, request->bu_type);
strncpy (sess->data_acb->bu_type, request->bu_type, sizeof(sess->data_acb->bu_type) - 1);
sess->data_acb->bu_type[sizeof(sess->data_acb->bu_type) - 1] = '\0';

error = data_copy_environment (sess,
request->env.env_val, request->env.env_len);
Expand Down Expand Up @@ -2930,7 +2933,7 @@ ndmp_sxa_log_file (struct ndm_session *sess,
ca->recover_log_file_error++;
}

sprintf (prefix, "%cLF", ref_conn->chan.name[1]);
snprintf (prefix, sizeof(prefix), "%cLF", ref_conn->chan.name[1]);

ndmalogf (sess, prefix, lev, "%s: %s", tag, request->name);
NDMS_ENDWITH
Expand Down Expand Up @@ -2960,7 +2963,7 @@ ndmp2_sxa_log_log (struct ndm_session *sess,
tag = "n";
lev = 1;

sprintf (prefix, "%cLM%s", ref_conn->chan.name[1], tag);
snprintf (prefix, sizeof(prefix), "%cLM%s", ref_conn->chan.name[1], tag);

bp = strrchr(request->entry, '\n');
if (bp) {
Expand Down Expand Up @@ -2991,7 +2994,7 @@ ndmp2_sxa_log_debug (struct ndm_session *sess,
tag = "d";
lev = 2;

sprintf (prefix, "%cLM%s", ref_conn->chan.name[1], tag);
snprintf (prefix, sizeof(prefix), "%cLM%s", ref_conn->chan.name[1], tag);

bp = strrchr(request->message, '\n');
if (bp) {
Expand Down Expand Up @@ -3051,7 +3054,7 @@ ndmp_sxa_log_message (struct ndm_session *sess,
break;
}

sprintf (prefix, "%cLM%s", ref_conn->chan.name[1], tag);
snprintf (prefix, sizeof(prefix), "%cLM%s", ref_conn->chan.name[1], tag);

bp = strrchr(request->entry, '\n');
if (bp) {
Expand Down
69 changes: 57 additions & 12 deletions src/ndmp/ndma_cops_query.c
View file
Expand Up @@ -178,6 +178,8 @@ int
ndmca_opq_host_info (struct ndm_session *sess, struct ndmconn *conn)
{
int rc;
int cnt;
int out;
unsigned int i;
char buf[100];

Expand All @@ -204,13 +206,20 @@ ndmca_opq_host_info (struct ndm_session *sess, struct ndmconn *conn)
ndmalogqr (sess, " hostid %s", reply->hostid);

*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->auth_type.auth_type_len; i++) {
ndmp2_auth_type atyp;

atyp = reply->auth_type.auth_type_val[i];
strcat (buf, " ");
strcat (buf, ndmp2_auth_type_to_str (atyp));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp2_auth_type_to_str (atyp));
else
rc = snprintf(buf + out, cnt, "%s", ndmp2_auth_type_to_str (atyp));
out += rc;
cnt -= rc;
}
buf[sizeof(buf) - 1] = '\0';

ndmalogqr (sess, " auths (%d) %s",
reply->auth_type.auth_type_len, buf);
Expand Down Expand Up @@ -253,13 +262,21 @@ ndmca_opq_host_info (struct ndm_session *sess, struct ndmconn *conn)
ndmalogqr (sess, " product %s", reply->product_name);
ndmalogqr (sess, " revision %s", reply->revision_number);
*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->auth_type.auth_type_len; i++) {
ndmp3_auth_type atyp;

atyp = reply->auth_type.auth_type_val[i];
strcat (buf, " ");
strcat (buf, ndmp3_auth_type_to_str (atyp));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp3_auth_type_to_str (atyp));
else
rc = snprintf(buf + out, cnt, "%s", ndmp3_auth_type_to_str (atyp));
out += rc;
cnt -= rc;
}
buf[sizeof(buf) - 1] = '\0';

ndmalogqr (sess, " auths (%d) %s",
reply->auth_type.auth_type_len, buf);
ndmalogqr (sess, "");
Expand Down Expand Up @@ -301,13 +318,21 @@ ndmca_opq_host_info (struct ndm_session *sess, struct ndmconn *conn)
ndmalogqr (sess, " product %s", reply->product_name);
ndmalogqr (sess, " revision %s", reply->revision_number);
*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->auth_type.auth_type_len; i++) {
ndmp4_auth_type atyp;

atyp = reply->auth_type.auth_type_val[i];
strcat (buf, " ");
strcat (buf, ndmp4_auth_type_to_str (atyp));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp4_auth_type_to_str (atyp));
else
rc = snprintf(buf + out, cnt, "%s", ndmp4_auth_type_to_str (atyp));
out += rc;
cnt -= rc;
}
buf[sizeof(buf) - 1] = '\0';

ndmalogqr (sess, " auths (%d) %s",
reply->auth_type.auth_type_len, buf);
ndmalogqr (sess, "");
Expand All @@ -325,6 +350,8 @@ int
ndmca_opq_get_mover_type (struct ndm_session *sess, struct ndmconn *conn)
{
int rc;
int cnt;
int out;
unsigned int i;
char buf[100];

Expand All @@ -345,12 +372,18 @@ ndmca_opq_get_mover_type (struct ndm_session *sess, struct ndmconn *conn)
ndmalogqr (sess, " Mover types");

*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->methods.methods_len; i++) {
ndmp2_mover_addr_type val;

val = reply->methods.methods_val[i];
strcat (buf, " ");
strcat (buf, ndmp2_mover_addr_type_to_str (val));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp2_mover_addr_type_to_str (val));
else
rc = snprintf(buf + out, cnt, "%s", ndmp2_mover_addr_type_to_str (val));
out += rc;
cnt -= rc;
}
ndmalogqr (sess, " methods (%d) %s",
reply->methods.methods_len,
Expand All @@ -373,12 +406,18 @@ ndmca_opq_get_mover_type (struct ndm_session *sess, struct ndmconn *conn)

ndmalogqr (sess, " Connection types");
*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->addr_types.addr_types_len; i++) {
ndmp3_addr_type val;

val = reply->addr_types.addr_types_val[i];
strcat (buf, " ");
strcat (buf, ndmp3_addr_type_to_str (val));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp3_addr_type_to_str (val));
else
rc = snprintf(buf + out, cnt, "%s", ndmp3_addr_type_to_str (val));
out += rc;
cnt -= rc;
}
ndmalogqr (sess, " addr_types (%d) %s",
reply->addr_types.addr_types_len, buf);
Expand All @@ -400,12 +439,18 @@ ndmca_opq_get_mover_type (struct ndm_session *sess, struct ndmconn *conn)

ndmalogqr (sess, " Connection types");
*buf = 0;
out = 0;
cnt = sizeof(buf) - 1;
for (i = 0; i < reply->addr_types.addr_types_len; i++) {
ndmp4_addr_type val;

val = reply->addr_types.addr_types_val[i];
strcat (buf, " ");
strcat (buf, ndmp4_addr_type_to_str (val));
if (out)
rc = snprintf(buf + out, cnt, " %s", ndmp4_addr_type_to_str (val));
else
rc = snprintf(buf + out, cnt, "%s", ndmp4_addr_type_to_str (val));
out += rc;
cnt -= rc;
}
ndmalogqr (sess, " addr_types (%d) %s",
reply->addr_types.addr_types_len, buf);
Expand Down
2 changes: 1 addition & 1 deletion src/ndmp/ndma_cops_robot.c
View file
Expand Up @@ -368,7 +368,7 @@ ndmca_op_unload_tape (struct ndm_session *sess)
goto unload_anyway;
}

sprintf (prefix, "drive @%d full", edp->element_address);
snprintf (prefix, sizeof(prefix), "drive @%d full", edp->element_address);

if (!edp->SValid) {
ndmalogf (sess, 0, 1,
Expand Down
2 changes: 1 addition & 1 deletion src/ndmp/ndma_ctrl_media.c
View file
Expand Up @@ -453,7 +453,7 @@ ndmca_media_write_label (struct ndm_session *sess, int type, char labbuf[])
for (p = buf; p < &buf[512]; p++) *p = '#';
for (p = buf+63; p < &buf[512]; p += 64) *p = '\n';

sprintf (buf, "##ndmjob -%c %s", type, labbuf);
snprintf (buf, sizeof(buf), "##ndmjob -%c %s", type, labbuf);
for (p = buf; *p; p++) continue;
*p++ = '\n';

Expand Down
6 changes: 3 additions & 3 deletions src/ndmp/ndma_ctrl_robot.c
View file
Expand Up @@ -311,7 +311,7 @@ ndmca_robot_remedy_ready (struct ndm_session *sess)
if (!edp->Full)
continue;

sprintf (prefix, "drive @%d not empty", edp->element_address);
snprintf (prefix, sizeof(prefix), "drive @%d not empty", edp->element_address);

if (!edp->SValid) {
ndmalogf (sess, 0, 1, "%s, invalid source", prefix);
Expand Down Expand Up @@ -409,9 +409,9 @@ ndmca_robot_query (struct ndm_session *sess)
lineno, buf);

if (lineno == 0)
sprintf (lnbuf, " %2d ", i+1);
snprintf (lnbuf, sizeof(lnbuf), " %2d ", i+1);
else
sprintf (lnbuf, " ");
snprintf (lnbuf, sizeof(lnbuf), " ");

if (rc < 0) {
strcpy (buf, "PP-ERROR");
Expand Down
6 changes: 3 additions & 3 deletions src/ndmp/ndma_ctst_data.c
View file
Expand Up @@ -369,7 +369,7 @@ ndmca_test_check_data_state (struct ndm_session *sess,

what = "state";
if (ds->state != expected) {
sprintf (errbuf, "expected %s got %s",
snprintf (errbuf, sizeof(errbuf), "expected %s got %s",
ndmp9_data_state_to_str (expected),
ndmp9_data_state_to_str (ds->state));
goto fail;
Expand All @@ -379,7 +379,7 @@ ndmca_test_check_data_state (struct ndm_session *sess,
switch (ds->state) {
case NDMP9_DATA_STATE_HALTED:
if (ds->halt_reason != (ndmp9_data_halt_reason)reason) {
sprintf (errbuf, "expected %s got %s",
snprintf (errbuf, sizeof(errbuf), "expected %s got %s",
ndmp9_data_halt_reason_to_str (reason),
ndmp9_data_halt_reason_to_str (ds->halt_reason));
goto fail;
Expand All @@ -397,7 +397,7 @@ ndmca_test_check_data_state (struct ndm_session *sess,

fail:
/* test failed */
sprintf(tmpbuf, "%s: %s", what, errbuf);
snprintf(tmpbuf, sizeof(tmpbuf), "%s: %s", what, errbuf);
ndmca_test_fail(sess, tmpbuf);

ndmca_test_close (sess);
Expand Down
8 changes: 4 additions & 4 deletions src/ndmp/ndma_ctst_mover.c
View file
Expand Up @@ -553,7 +553,7 @@ ndmca_test_check_mover_state (struct ndm_session *sess,

what = "state";
if (ms->state != expected) {
sprintf (errbuf, "expected %s got %s",
snprintf (errbuf, sizeof(errbuf), "expected %s got %s",
ndmp9_mover_state_to_str (expected),
ndmp9_mover_state_to_str (ms->state));
goto fail;
Expand All @@ -563,7 +563,7 @@ ndmca_test_check_mover_state (struct ndm_session *sess,
switch (ms->state) {
case NDMP9_MOVER_STATE_PAUSED:
if (ms->pause_reason != (ndmp9_mover_pause_reason)reason) {
sprintf (errbuf, "expected %s got %s",
snprintf (errbuf, sizeof(errbuf), "expected %s got %s",
ndmp9_mover_pause_reason_to_str (reason),
ndmp9_mover_pause_reason_to_str (ms->pause_reason));
goto fail;
Expand All @@ -572,7 +572,7 @@ ndmca_test_check_mover_state (struct ndm_session *sess,

case NDMP9_MOVER_STATE_HALTED:
if (ms->halt_reason != (ndmp9_mover_halt_reason)reason) {
sprintf (errbuf, "expected %s got %s",
snprintf (errbuf, sizeof(errbuf), "expected %s got %s",
ndmp9_mover_halt_reason_to_str (reason),
ndmp9_mover_halt_reason_to_str (ms->halt_reason));
goto fail;
Expand All @@ -590,7 +590,7 @@ ndmca_test_check_mover_state (struct ndm_session *sess,

fail:
/* test failed */
sprintf(tmpbuf, "%s: %s", what, errbuf);
snprintf(tmpbuf, sizeof(tmpbuf), "%s: %s", what, errbuf);
ndmca_test_fail(sess, tmpbuf);

ndmca_test_close (sess);
Expand Down
6 changes: 3 additions & 3 deletions src/ndmp/ndma_ctst_subr.c
View file
Expand Up @@ -223,7 +223,7 @@ ndmca_test_check_expect_errs (struct ndmconn *conn, int rc,
ndmp9_error_to_str (expect_errs[i]));
}

sprintf(tmpbuf, "got %s (error expected)", ndmp9_error_to_str (reply_error));
snprintf(tmpbuf, sizeof(tmpbuf), "got %s (error expected)", ndmp9_error_to_str (reply_error));

if (rc == 2)
ndmca_test_warn (sess, tmpbuf);
Expand Down Expand Up @@ -307,7 +307,7 @@ ndmca_test_call (struct ndmconn *conn,

if (rc != 0) {
char tmpbuf[128];
sprintf(tmpbuf, "got %s (call)", ndmp9_error_to_str (reply_error));
snprintf(tmpbuf, sizeof(tmpbuf), "got %s (call)", ndmp9_error_to_str (reply_error));
if (rc == 2)
ndmca_test_warn (sess, tmpbuf);
else
Expand All @@ -333,7 +333,7 @@ ndmca_test_open (struct ndm_session *sess, char *test_name, char *sub_test_name)
if (sess->control_acb->active_test == 0) {
/* record name */
if (sub_test_name)
sprintf(test_name_buf, "%s/%s", test_name, sub_test_name);
snprintf(test_name_buf, sizeof(test_name_buf), "%s/%s", test_name, sub_test_name);
else
strcpy(test_name_buf, test_name);
sess->control_acb->active_test = test_name_buf;
Expand Down

0 comments on commit 839923a

Please sign in to comment.