Re: [ovs-dev] [PATCH] lib, ovsdb, ovs-vsctl, vtep-ctl: Fix multiple Coverity defects
Bleep bloop. Greetings James Raphael Tiovalen, I am a robot and I have tried
out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#231 FILE: lib/dpctl.c:470:
dpctl_error(dpctl_p, 0, "%s: can't change port number
from"
WARNING: Line is 80 characters long (recommended limit is 79)
#232 FILE: lib/dpctl.c:471:
" %"PRIu32" to %d", name, port_no, atoi(value));
WARNING: Line is 80 characters long (recommended limit is 79)
#502 FILE: lib/odp-execute.c:184:
nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_ttl << 8),
WARNING: Line is 96 characters long (recommended limit is 79)
#617 FILE: lib/sflow_agent.c:158:
/* add to end of list - to preserve the receiver index numbers for
existing receivers */
ERROR: Improper whitespace around control block
#620 FILE: lib/sflow_agent.c:161:
for(r = agent->receivers; r != NULL; prev = r, r = r->nxt);
ERROR: Improper whitespace around control block
#621 FILE: lib/sflow_agent.c:162:
if(prev) prev->nxt = rcv;
ERROR: Inappropriate spacing around cast
#649 FILE: lib/sflow_agent.c:208:
SFLSampler *newsm = (SFLSampler *)sflAlloc(agent, sizeof(SFLSampler));
ERROR: Improper whitespace around control block
#653 FILE: lib/sflow_agent.c:212:
if(prev) prev->nxt = newsm;
WARNING: Line is 98 characters long (recommended limit is 79)
#659 FILE: lib/sflow_agent.c:218:
SFLSampler *test = sfl_agent_getSamplerByIfIndex(agent,
SFL_DS_INDEX(newsm->dsi));
WARNING: Line is 89 characters long (recommended limit is 79)
#660 FILE: lib/sflow_agent.c:219:
if (test && (SFL_DS_INSTANCE(newsm->dsi) <
SFL_DS_INSTANCE(test->dsi))) {
WARNING: Line is 93 characters long (recommended limit is 79)
#661 FILE: lib/sflow_agent.c:220:
/* replace with this new one because it has a lower
ds_instance number */
ERROR: Inappropriate bracing around statement
#665 FILE: lib/sflow_agent.c:224:
if (test == NULL) sfl_agent_jumpTableAdd(agent, newsm);
ERROR: Inappropriate spacing around cast
#682 FILE: lib/sflow_agent.c:249:
SFLPoller *newpl = (SFLPoller *)sflAlloc(agent, sizeof(SFLPoller));
ERROR: Inappropriate bracing around statement
#686 FILE: lib/sflow_agent.c:253:
if (prev) prev->nxt = newpl;
WARNING: Line is 83 characters long (recommended limit is 79)
#774 FILE: ovsdb/file.c:543:
json_object_put(ftxn->json, table->schema->name,
ftxn->table_json);
WARNING: Line is 101 characters long (recommended limit is 79)
#947 FILE: ovsdb/ovsdb-client.c:1237:
const struct ovsdb_column *version_column =
ovsdb_table_schema_get_column(table, "_version");
WARNING: Line is 82 characters long (recommended limit is 79)
#1079 FILE: ovsdb/ovsdb.c:222:
return ovsdb_syntax_error(json, NULL, "schema version \"%s\"
not "
WARNING: Line is 82 characters long (recommended limit is 79)
#1104 FILE: ovsdb/ovsdb.c:239:
error = ovsdb_syntax_error(json, NULL, "name must be a valid
id");
WARNING: Line is 82 characters long (recommended limit is 79)
#1142 FILE: ovsdb/ovsdb.c:256:
/* "isRoot" was not part of the original schema definition. Before it
was
WARNING: Line is 81 characters long (recommended limit is 79)
#1144 FILE: ovsdb/ovsdb.c:258:
* compatibility, if the root set is empty then assume that every table
is
WARNING: Line is 82 characters long (recommended limit is 79)
#1166 FILE: ovsdb/ovsdb.c:271:
* ovsdb_schema_check_ref_table(). This requires 'is_root' to be known,
so
WARNING: Line is 81 characters long (recommended limit is 79)
#1189 FILE: ovsdb/ovsdb.c:283:
error = ovsdb_schema_check_ref_table(column,
>tables,
WARNING: Line is 80 characters long (recommended limit is 79)
#1309 FILE: utilities/ovs-vsctl.c:339:
ctl_fatal("value %s on -t or --timeout is invalid", optarg);
WARNING: Line is 80 characters long (recommended limit is 79)
#1418 FILE: utilities/ovs-vsctl.c:842:
VLOG_WARN("%s: port is in multiple bridges (%s and %s)",
WARNING: Line is 82 characters long (recommended limit is 79)
#1426 FILE: utilities/ovs-vsctl.c:846:
* uniqueness constraints, so the database server
shouldn't
WARNING: Line is 85 characters long (recommended limit is 79)
#1443 FILE: utilities/ovs-vsctl.c:862:
struct ovsrec_interface *iface_cfg =
port_cfg->interfaces[k];
WARNING: Line is 85 characters long (recommended limit is 79)
#1446 FILE: utilities/ovs-vsctl.c:865:
iface = shash_find_data(_ctx->ifaces,
iface_cfg->name);
WARNING: Line is 87 characters long (recommended limit is 79)
#1455 FILE: utilities/ovs-vsctl.c:874:
/* Log
[ovs-dev] [PATCH] lib, ovsdb, ovs-vsctl, vtep-ctl: Fix multiple Coverity defects
This commit addresses several high and medium-impact Coverity defects by
fixing several possible null-pointer dereferences and potentially
uninitialized variables.
Some variables and structs would always be initialized by
the code flow and some pointers would never be null, and thus, some
Coverity reports might be false positives. That said, it is still
considered best practice to perform null pointer checks and initialize
variables upfront just in case to ensure the overall security of OVS, as
long as they do not impact performance-critical code. As a bonus, it
would also make static analyzer tools, such as Coverity, happy.
Unit tests have been successfully executed via `make check` and they
successfully passed.
Signed-off-by: James Raphael Tiovalen
---
lib/dp-packet.c | 69 +++---
lib/dpctl.c | 64 +
lib/json.c | 32 +
lib/latch-unix.c| 2 +-
lib/memory.c| 10 +--
lib/netdev-native-tnl.c | 48 +++--
lib/odp-execute.c | 90
lib/pcap-file.c | 5 +-
lib/rtnetlink.c | 11 ++-
lib/sflow_agent.c | 70 ++-
lib/shash.c | 4 +-
lib/sset.c | 7 +-
ovsdb/condition.c | 10 ++-
ovsdb/file.c| 25 +--
ovsdb/jsonrpc-server.c | 5 +-
ovsdb/monitor.c | 49 ++---
ovsdb/ovsdb-client.c| 44
ovsdb/ovsdb-server.c| 15 ++--
ovsdb/ovsdb-util.c | 5 ++
ovsdb/ovsdb.c | 121
ovsdb/query.c | 4 +-
ovsdb/replication.c | 14 ++--
ovsdb/row.c | 4 +-
ovsdb/table.c | 2 +-
ovsdb/transaction.c | 8 ++-
utilities/ovs-vsctl.c | 150 +++-
vtep/vtep-ctl.c | 77 +++--
27 files changed, 566 insertions(+), 379 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ae8ab5800..50cb30681 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -171,7 +171,11 @@ dp_packet_new_with_headroom(size_t size, size_t headroom)
struct dp_packet *
dp_packet_clone(const struct dp_packet *buffer)
{
-return dp_packet_clone_with_headroom(buffer, 0);
+if (buffer) {
+return dp_packet_clone_with_headroom(buffer, 0);
+} else {
+return NULL;
+}
}
/* Creates and returns a new dp_packet whose data are copied from 'buffer'.
@@ -183,26 +187,32 @@ dp_packet_clone_with_headroom(const struct dp_packet
*buffer, size_t headroom)
struct dp_packet *new_buffer;
uint32_t mark;
-new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
- dp_packet_size(buffer),
- headroom);
-/* Copy the following fields into the returned buffer: l2_pad_size,
- * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
-memcpy(_buffer->l2_pad_size, >l2_pad_size,
-sizeof(struct dp_packet) -
-offsetof(struct dp_packet, l2_pad_size));
+const void *data_dp = dp_packet_data(buffer);
-*dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
-*dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
+if (data_dp) {
+new_buffer = dp_packet_clone_data_with_headroom(data_dp,
+dp_packet_size(buffer),
+headroom);
+/* Copy the following fields into the returned buffer: l2_pad_size,
+* l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
+memcpy(_buffer->l2_pad_size, >l2_pad_size,
+sizeof(struct dp_packet) -
+offsetof(struct dp_packet, l2_pad_size));
-if (dp_packet_rss_valid(buffer)) {
-dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
-}
-if (dp_packet_has_flow_mark(buffer, )) {
-dp_packet_set_flow_mark(new_buffer, mark);
-}
+*dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
+*dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
+
+if (dp_packet_rss_valid(buffer)) {
+dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
+}
+if (dp_packet_has_flow_mark(buffer, )) {
+dp_packet_set_flow_mark(new_buffer, mark);
+}
-return new_buffer;
+return new_buffer;
+} else {
+return NULL;
+}
}
/* Creates and returns a new dp_packet that initially contains a copy of the
@@ -323,8 +333,11 @@ dp_packet_shift(struct dp_packet *b, int delta)
if (delta != 0) {
char *dst = (char *) dp_packet_data(b) + delta;
-memmove(dst, dp_packet_data(b), dp_packet_size(b));
-dp_packet_set_data(b, dst);
+const void *data_dp = dp_packet_data(b);
+
