Explicit NULL dereference (src/backend/commands/tablecmds.c)

Started by Ranier Vilelaover 4 years ago2 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

Hi,

Per Coverity.
CID 1453114 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
53. var_deref_model: Passing null pointer child_expr to strcmp, which
dereferences it.

It is agreed that asserts should be used for error conditions that can
never occur in the release.
But with errors that can occur, using assert does not make sense.

Better to make sure that strcmp can be called without risk.
Meanwhile, fix the strcmp call signature (const char).

#include <stdio.h>
#include <string.h>

int main()
{
const char * s1="";
const char * s2="0";

if (strstr(s1, s2) != 0) {
printf("found");
} else {
printf("not found");
}
}
not found!

regards,
Ranier Vilela

Attachments:

fix_null_dereference_tablecmds.patchapplication/octet-stream; name=fix_null_dereference_tablecmds.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ebc62034d2..9992c6cbc5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14505,8 +14505,8 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
 			{
 				TupleConstr *child_constr = child_rel->rd_att->constr;
 				TupleConstr *parent_constr = parent_rel->rd_att->constr;
-				char	   *child_expr = NULL;
-				char	   *parent_expr = NULL;
+				const char   *child_expr = "";
+				const char   *parent_expr = "0";
 
 				Assert(child_constr != NULL);
 				Assert(parent_constr != NULL);
@@ -14522,7 +14522,6 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
 						break;
 					}
 				}
-				Assert(child_expr != NULL);
 
 				for (int i = 0; i < parent_constr->num_defval; i++)
 				{
@@ -14535,7 +14534,6 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
 						break;
 					}
 				}
-				Assert(parent_expr != NULL);
 
 				if (strcmp(child_expr, parent_expr) != 0)
 					ereport(ERROR,
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ranier Vilela (#1)
Re: Explicit NULL dereference (src/backend/commands/tablecmds.c)

Ranier Vilela <ranier.vf@gmail.com> writes:

It is agreed that asserts should be used for error conditions that can
never occur in the release.
But with errors that can occur, using assert does not make sense.

On what grounds do you claim that those asserts are wrong?

Coverity's opinion counts for just about nothing these days.
A test case causing a crash would count, of course.

regards, tom lane