[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD
From: |
Mark D. Baushke |
Subject: |
Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD |
Date: |
Fri, 12 May 2006 08:18:37 -0700 |
Hi Christos,
Christos Zoulas <address@hidden> writes:
> On May 12, 6:35am, address@hidden ("Mark D. Baushke") wrote:
> -- Subject: Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD
>
> | Would it make sense to modify src/gnu/usr.bin/cvs/include/config.h
> | so that it does a
> |
> | /* Functions defined in src/gnu/dist/xcvs/src/subr.c */
> | #define xasprintf cvs_xasprintf
> | #define xmalloc cvs_xmalloc
> | #define xrealloc cvs_xrealloc
> | #define xstrdup cvs_xstrdup
> |
> | or, does the coverity model processing not handle macro processing
> | correctly?
>
> It does, and it will work properly [provided it compiles :-)]
You might give it a try and see if it works for you.
> | For what it is worth, I have not finished going through all of the run
> | 22 reported problems. I know that there is at least a few more that are
> | 'real' bugs (cid-1058 and cid-1061 are both such).
> |
> | Given that our xstrdup() is safe, I will probably be considering reverting
> | the=20
> |
> | var = str ? xstrdup (str) : NULL;
> |
> | changes back into
> |
> | var = xstrdup (str);
>
> Yes, we should revert the changes.
Agreed. The patch below reverts those changes and adds a few asserts to
deal with the other cases. I have committed it to the CVS STABLE branch
(1.11.x).
> | in the CVS STABLE and CVS FEATURE source bases. (It turns out that CVS
> | FEATURE uses Xstrdup to do the NULL check before calling the real
> | xstrdup () function, but has a '#define xstrdup Xstrdup' to hide it.)
>
> Ouch, pre-processor macro hell...
Indeed.
-- Mark
Index: ChangeLog
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/ChangeLog,v
retrieving revision 1.2
diff -u -p -r1.2 ChangeLog
--- ChangeLog 4 May 2006 15:39:34 -0000 1.2
+++ ChangeLog 12 May 2006 14:28:53 -0000
@@ -1,18 +1,67 @@
+2006-05-12 Mark D. Baushke <address@hidden>
+
+ * rcs.c (RCS_isdead): Assert that the first argument is not NULL.
+ [Fixes NetBSD cid-1058.]
+
+ * commit.c (checkaddfile): Do not dereference NULL on call to
+ error().
+ [Fixes NetBSD cid-1061.]
+
+ * log.c (cvslog): Assert p->start && p->end instead of masking the
+ problem.
+ * server.c (server_updated): Assert findnode_fn results instead of
+ masking the problem.
+
+ * add.c (add_directory): Revert previous change. The xstrdup()
+ function already deals a NULL argument.
+ * client.c (handle_mt): Ditto.
+ * entries.c (Entnode_Create): Ditto.
+ (Entries_Open): Ditto.
+ * logmsg.c (fmt_proc): Ditto.
+ * vers_ts.c (Version_TS): Ditto.
+
+2006-05-11 Mark D. Baushke <address@hidden>
+
+ * add.c (add_directory): Protect tag from NULL dereference.
+ [Fixes NetBSD cid-1054.]
+
+ * client.c (handle_mt): Deal with missing text argument.
+ [Fixes NetBSD cid-924.]
+
+ * entries.c (Entnode_Create): Protect date, tag and ts_conflict
+ from possible NULL dereference.
+ [Fixes NetBSD coverity cid-994, cid-995, cid-1055, cid-1057.]
+
+ * entries.c (Entries_Open): Protect dirtag and dirdate from
+ possible NULL dereference.
+ [Fixes NetBSD coverity cid-996.]
+
+ * log.c (cvslog): Validate start and end args to
+ date_to_internet().
+ [Fixes NetBSD coverity cid-2427 and cid-2428.]
+
+ * logmsg.c (fmt_proc): Protect li->tag from NULL dereference.
+ [Fixes NetBSD coverity cid-997.]
+
+ * vers_ts.c (Version_TS): Protect tag and vers_ts->tag from NULL
+ dereference.
+ [Fixes NetBSD coverity cid-1053.]
+
2006-05-04 Mark D. Baushke <address@hidden>
- * filesubr.c (cvs_temp_file): Avoid keeping pointers to free()'d
- storage laying around.
- * commit.c (commit): Handle possible NULL filename values
- returned from cvs_temp_file().
- * filesubr.c (cvs_temp_name): Ditto.
- * import.c (import): Ditto.
- * login.c (password_entry_operation): Ditto.
- * logmsg.c (do_verify): Ditto.
- * patch.c (patch_fileproc): Ditto.
- [Fixes NetBSD coverity cid-2545.]
+ * filesubr.c (cvs_temp_file): Avoid keeping pointers to free()'d
+ storage laying around.
+ * commit.c (commit): Handle possible NULL filename values
+ returned from cvs_temp_file().
+ * filesubr.c (cvs_temp_name): Ditto.
+ * import.c (import): Ditto.
+ * login.c (password_entry_operation): Ditto.
+ * logmsg.c (do_verify): Ditto.
+ * patch.c (patch_fileproc): Ditto.
+ [Fixes NetBSD coverity cid-2545.]
- * buffer.c (packetizing_buffer_output): Initialize outdata.
- [Fixes NetBSD coverity cid-2474.]
+ * buffer.c (packetizing_buffer_output): Initialize outdata.
+ [Fixes NetBSD coverity cid-2474.]
* server.c (server_updated): Fix NetBSD coverity cid-1352
NetBSD-sparc64 of 2006-May-02 03:02:46.
Index: add.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/add.c,v
retrieving revision 1.2
diff -u -p -r1.2 add.c
--- add.c 11 May 2006 17:47:49 -0000 1.2
+++ add.c 12 May 2006 14:28:53 -0000
@@ -852,7 +852,7 @@ add_directory (finfo)
p->key = xstrdup ("- New directory");
li = (struct logfile_info *) xmalloc (sizeof (struct logfile_info));
li->type = T_TITLE;
- li->tag = tag ? xstrdup (tag) : NULL;
+ li->tag = xstrdup (tag);
li->rev_old = li->rev_new = NULL;
p->data = li;
(void) addnode (ulist, p);
Index: client.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/client.c,v
retrieving revision 1.4
diff -u -p -r1.4 client.c
--- client.c 11 May 2006 17:47:49 -0000 1.4
+++ client.c 12 May 2006 14:28:54 -0000
@@ -3277,7 +3277,7 @@ handle_mt (args, len)
cvs_output ("\n", 1);
free (updated_fname);
}
- updated_fname = text ? xstrdup (text) : NULL;
+ updated_fname = xstrdup (text);
}
/* Swallow all other tags. Either they are extraneous
or they reflect future extensions that we can
@@ -3288,11 +3288,11 @@ handle_mt (args, len)
if (strcmp (tag, "conflicts") == 0)
importmergecmd.conflicts = text ? atoi (text) : -1;
else if (strcmp (tag, "mergetag1") == 0)
- importmergecmd.mergetag1 = text ? xstrdup (text) : NULL;
+ importmergecmd.mergetag1 = xstrdup (text);
else if (strcmp (tag, "mergetag2") == 0)
- importmergecmd.mergetag2 = text ? xstrdup (text) : NULL;
+ importmergecmd.mergetag2 = xstrdup (text);
else if (strcmp (tag, "repository") == 0)
- importmergecmd.repository = text ? xstrdup (text) : NULL;
+ importmergecmd.repository = xstrdup (text);
/* Swallow all other tags. Either they are text for
which we are going to print our own version when we
see -importmergecmd, or they are future extensions
Index: commit.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/commit.c,v
retrieving revision 1.3
diff -u -p -r1.3 commit.c
--- commit.c 4 May 2006 15:39:34 -0000 1.3
+++ commit.c 12 May 2006 14:28:54 -0000
@@ -2127,7 +2127,7 @@ checkaddfile (file, repository, tag, opt
rcs = RCS_parse (file, repository);
if (rcs == NULL)
{
- error (0, 0, "could not read %s", rcs->path);
+ error (0, 0, "could not read %s in %s", file, repository);
goto out;
}
*rcsnode = rcs;
Index: entries.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/entries.c,v
retrieving revision 1.3
diff -u -p -r1.3 entries.c
--- entries.c 11 May 2006 17:47:49 -0000 1.3
+++ entries.c 12 May 2006 14:28:54 -0000
@@ -59,9 +59,9 @@ Entnode_Create(type, user, vn, ts, optio
ent->version = xstrdup (vn);
ent->timestamp = xstrdup (ts ? ts : "");
ent->options = xstrdup (options ? options : "");
- ent->tag = tag ? xstrdup (tag) : NULL;
- ent->date = date ? xstrdup (date) : NULL;
- ent->conflict = ts_conflict ? xstrdup (ts_conflict) : NULL;
+ ent->tag = xstrdup (tag);
+ ent->date = xstrdup (date);
+ ent->conflict = xstrdup (ts_conflict);
return ent;
}
@@ -491,8 +491,8 @@ Entries_Open (aflag, update_dir)
sdtp = (struct stickydirtag *) xmalloc (sizeof (*sdtp));
memset ((char *) sdtp, 0, sizeof (*sdtp));
sdtp->aflag = aflag;
- sdtp->tag = dirtag ? xstrdup (dirtag) : NULL;
- sdtp->date = dirdate ? xstrdup (dirdate) : NULL;
+ sdtp->tag = xstrdup (dirtag);
+ sdtp->date = xstrdup (dirdate);
sdtp->nonbranch = dirnonbranch;
/* feed it into the list-private area */
Index: log.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/log.c,v
retrieving revision 1.2
diff -u -p -r1.2 log.c
--- log.c 11 May 2006 17:47:49 -0000 1.2
+++ log.c 12 May 2006 14:28:54 -0000
@@ -316,39 +316,33 @@ cvslog (argc, argv)
{
p = log_data.datelist;
log_data.datelist = p->next;
- if (p->start != NULL && p->end != NULL)
- {
- send_to_server ("Argument -d\012", 0);
- send_to_server ("Argument ", 0);
- date_to_internet (datetmp, p->start);
- send_to_server (datetmp, 0);
- if (p->inclusive)
- send_to_server ("<=", 0);
- else
- send_to_server ("<", 0);
- date_to_internet (datetmp, p->end);
- send_to_server (datetmp, 0);
- send_to_server ("\012", 0);
- }
- if (p->start)
- free (p->start);
- if (p->end)
- free (p->end);
+ assert (p->start != NULL && p->end != NULL);
+ send_to_server ("Argument -d\012", 0);
+ send_to_server ("Argument ", 0);
+ date_to_internet (datetmp, p->start);
+ send_to_server (datetmp, 0);
+ if (p->inclusive)
+ send_to_server ("<=", 0);
+ else
+ send_to_server ("<", 0);
+ date_to_internet (datetmp, p->end);
+ send_to_server (datetmp, 0);
+ send_to_server ("\012", 0);
+ free (p->start);
+ free (p->end);
free (p);
}
while (log_data.singledatelist != NULL)
{
p = log_data.singledatelist;
log_data.singledatelist = p->next;
- if (p->end)
- {
- send_to_server ("Argument -d\012", 0);
- send_to_server ("Argument ", 0);
- date_to_internet (datetmp, p->end);
- send_to_server (datetmp, 0);
- send_to_server ("\012", 0);
- free (p->end);
- }
+ assert (p->end != NULL);
+ send_to_server ("Argument -d\012", 0);
+ send_to_server ("Argument ", 0);
+ date_to_internet (datetmp, p->end);
+ send_to_server (datetmp, 0);
+ send_to_server ("\012", 0);
+ free (p->end);
free (p);
}
Index: logmsg.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/logmsg.c,v
retrieving revision 1.4
diff -u -p -r1.4 logmsg.c
--- logmsg.c 11 May 2006 17:47:49 -0000 1.4
+++ logmsg.c 12 May 2006 14:28:54 -0000
@@ -155,7 +155,7 @@ fmt_proc (p, closure)
if (tag != NULL)
free (tag);
- tag = li->tag ? xstrdup (li->tag) : NULL;
+ tag = xstrdup (li->tag);
/* Force a new line. */
col = 70;
Index: rcs.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/rcs.c,v
retrieving revision 1.2
diff -u -p -r1.2 rcs.c
--- rcs.c 4 Feb 2006 16:29:56 -0000 1.2
+++ rcs.c 12 May 2006 14:28:54 -0000
@@ -3487,6 +3487,8 @@ RCS_isdead (rcs, tag)
Node *p;
RCSVers *version;
+ assert (rcs != NULL);
+
if (rcs->flags & PARTIAL)
RCS_reparsercsfile (rcs, (FILE **) NULL, (struct rcsbuffer *) NULL);
Index: server.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/server.c,v
retrieving revision 1.4
diff -u -p -r1.4 server.c
--- server.c 4 May 2006 15:39:34 -0000 1.4
+++ server.c 12 May 2006 14:28:54 -0000
@@ -4235,6 +4235,7 @@ CVS server internal error: no mode in se
in case we end up processing it again (e.g. modules3-6
in the testsuite). */
node = findnode_fn (finfo->entries, finfo->file);
+ assert (node != NULL);
if (node != NULL)
{
Entnode *entnode = node->data;
Index: vers_ts.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/vers_ts.c,v
retrieving revision 1.2
diff -u -p -r1.2 vers_ts.c
--- vers_ts.c 11 May 2006 17:47:49 -0000 1.2
+++ vers_ts.c 12 May 2006 14:28:54 -0000
@@ -155,8 +155,8 @@ Version_TS (finfo, options, tag, date, f
*/
if (tag || date)
{
- vers_ts->tag = tag ? xstrdup (tag) : NULL;
- vers_ts->date = date ? xstrdup (date) : NULL;
+ vers_ts->tag = xstrdup (tag);
+ vers_ts->date = xstrdup (date);
}
else if (!vers_ts->entdata && (sdtp && sdtp->aflag == 0))
{
@@ -200,7 +200,7 @@ Version_TS (finfo, options, tag, date, f
if (vers_ts->vn_rcs == NULL)
vers_ts->vn_tag = NULL;
else if (simple)
- vers_ts->vn_tag = vers_ts->tag ? xstrdup (vers_ts->tag) : NULL;
+ vers_ts->vn_tag = xstrdup (vers_ts->tag);
else
vers_ts->vn_tag = xstrdup (vers_ts->vn_rcs);
}
- [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Mark D. Baushke, 2006/05/11
- [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Christos Zoulas, 2006/05/12
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Mark D. Baushke, 2006/05/12
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Christos Zoulas, 2006/05/15
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Mark D. Baushke, 2006/05/12
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Christos Zoulas, 2006/05/15
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD,
Mark D. Baushke <=
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Christos Zoulas, 2006/05/15
- Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD, Mark D. Baushke, 2006/05/12