Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[Martin S. Tignor]

On Tue, 2003-12-09 at 05:40, Marco Morandini wrote:
> This patch should take care of the
> 
> "delete of something allocated with new[]"
> 
> part of
> 
> http://opendx.watson.ibm.com/dx/mailArchives/mails.html/opendx-dev.0310/msg5.html
> 
> 
> Regards,
> 
> Marco Morandini
> 
> 
OK, I applied these patches.  I think code changes like these,
that address new and new[] matching up with delete and delete[] 
are not worthwhile.  Although they're fine changes, the
places where the code uses delete on the output of (something
like) DuplicateString() are so numerous that it would take
probably a dozen changes in every source file to fix.

I think Dave T. said something about making some of these
changes due compilation issues.  I don't remember if
he said it was an error or a warning he was dealing with
but he seemed to have a good reason to make the change.
But, I wouldn't make these changes if the purpose is to
address runtime warnings.

In short, the code ought to be different but changing
it is a lot of buck for no bang.

The ui code routinely does things like the following:
char* ptr = new char[10];
.
.
.
delete ptr;

...and other types show up there also, int, void*, whatever.
We just have to count on that working.  I've never seen it
not work.

It would certainly be wrong to instantiate an array of objects 
and use delete to get rid of them because that wouldn't call the
destructor for each object.  ...but I think the dxui code never 
instantiates arrays of objects as in
Base* bPtr = new Base[10];

(I'm overloading the definition of "never" to mean  probably not at all
but even if there is such a situation, the deletion is handled
correctly.)

Sort of on this topic, the code provides implementations of new and
delete operators (although not new[] and delete[] operators).   I had
thought that they should go away although I saw that we sometimes 
realloc the output of new (which is defined as malloc).  So I guess I
would leave it alone until there is a compelling reason to change it.
But there won't ever be one.

Attached is a valgrind suppressions file that (I think) does a good job
on the delete/delete[] warnings:
##--##

# Errors to suppress by default for glibc 2.1.3

# Format of this file is:
# {
# name_of_suppression
# kind: one of Param Value1 Value2 Value4 Value8
#   Free Addr1 Addr2 Addr4 Addr8
#   Cond (previously known as Value0)
# (if Param: name of system call param, if Free: name of free-ing fn)
# caller0 name, or /name/of/so/file.so
# caller1 name, or ditto
# (optionally: caller2 name)
# (optionally: caller3 name)
#  }

# Suppress errors appearing as a result of calling
# __libc_freeres()

{
   Mismatched free() / delete / delete [](Free)
   Free
   fun:*free*
   fun:*
}

{
   Mismatched free() / delete / delete [](Free)
   Free
   fun:*realloc*
   fun:*
}

Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[David Thompson]

If you look at the 1.13-1.12 diff of MsgWin.C, you'll see that I did 
change one delete (there were a whole bunch in that set of commits at 
that time). All of the delete's I fixed were the ones that were 
automatically detected by gcc 3.2. The problems they were having were 
with gcc complaining about them or not compiling because of them. I 
know that gcc 3.3 has even gotten better at detecting some of these 
but don't think its as good as something like purify or lint. I 
figure we should fix them just to keep code as clean as possible, but 
I don't always have the time. As people point them out, I will 
evaluate them and change the code--but since it really doesn't affect 
the code (for most compilers), I just have better things to do.


David




On Tue, 2003-12-09 at 12:55, Marco Morandini wrote:

 David L Thompson wrote:
 > These have already been rolled into 4.3.2.
 >

 Perhaps I'm wrong (or the patch is simply wrong),
 but I don't think so.

I guess I also don't follow.  ...probably just
making the same CVS mistake as Marco.

My thinking was like this:  We always used to do supply
operators new and delete although we didn't do
anything for new[] and delete[].  I never
paid much attention I just developed immunity to
'Mismatched free/delete' warnings that
purify issued. (Maybe unrelated.) So I was just going to go back and
see what the code said (and see if I could understand it).
Perhaps new[] and delete[] should have
been added.  Perhaps new and delete shouldn't have
been.  Perhaps... whatever.

However, among solutions, my preference is the kind that's already
checked in.

At any rate, my version of MsgWin.C (for example) uses delete
instead of delete[] just as Marco's does.

Martin



--
.
David L. Thompson   Visualization and Imagery Solutions, Inc.
mailto:[EMAIL PROTECTED]5515 Skyway Drive, Missoula, MT 59804
Phone : (406)756-7472

Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[Martin S. Tignor]

On Tue, 2003-12-09 at 12:55, Marco Morandini wrote:
> David L Thompson wrote:
> > These have already been rolled into 4.3.2.
> > 
> 
> Perhaps I'm wrong (or the patch is simply wrong),
> but I don't think so.
I guess I also don't follow.  ...probably just
making the same CVS mistake as Marco.

My thinking was like this:  We always used to do supply
operators new and delete although we didn't do
anything for new[] and delete[].  I never
paid much attention I just developed immunity to 
'Mismatched free/delete' warnings that
purify issued. (Maybe unrelated.) So I was just going to go back and
see what the code said (and see if I could understand it).
Perhaps new[] and delete[] should have
been added.  Perhaps new and delete shouldn't have
been.  Perhaps... whatever.

However, among solutions, my preference is the kind that's already
checked in.

At any rate, my version of MsgWin.C (for example) uses delete
instead of delete[] just as Marco's does.

Martin

Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[Marco Morandini]

David L Thompson wrote:

These have already been rolled into 4.3.2.



Perhaps I'm wrong (or the patch is simply wrong),
but I don't think so.

For example:

[EMAIL PROTECTED]:~/Programmi/dx/src/uipp/dxuilib> cvs diff -u -r HEAD MsgWin.C
Index: MsgWin.C
===
RCS file: /src/master/dx/src/uipp/dxuilib/MsgWin.C,v
retrieving revision 1.13
diff -u -r1.13 MsgWin.C
--- MsgWin.C30 Sep 2003 17:41:48 -  1.13
+++ MsgWin.C9 Dec 2003 17:47:35 -
@@ -584,7 +584,7 @@
XmStringFree(nameString);
XmStringFree(firstHalf);
XmStringFree(text);
-   delete line;
+   delete[] line;

XmListAddItemUnselected(this->list, s, 0);
XmStringFree(s);
@@ -702,7 +702,7 @@
 SelectableLine *l;
 while( (l = (SelectableLine*)li.getNext()) )
 {
-   delete l->line;
+   delete[] l->line;
delete l;
 }
 this->selectableLines.clear();
@@ -774,7 +774,7 @@
fputc('\n', this->logFile);
}
items[i] = XmStringCreate(s, "normal");
-   delete s;
+   delete[] s;
 }

 XmListAddItems(this->list, items, nItems, 0);
@@ -788,7 +788,7 @@

 for (i = 0; i < nItems; ++i)
XmStringFree(items[i]);
-delete items;
+delete[] items;

 this->batchedLines.clear();

@@ -814,7 +814,7 @@
 char *s;
 ListIterator li(this->batchedLines);
 for (int i = 0; i < nItems && (s = (char*)li.getNext()); ++i)
-   delete s;
+   delete[] s;
 this->batchedLines.clear();

 this->clearSelectableLines();
@@ -833,7 +833,7 @@
 if (file == NULL && this->logFile)
 {
fclose(this->logFile);
-   delete this->logFileName;
+   delete[] this->logFileName;
this->logFile = NULL;
this->logFileName = NULL;
this->logOption->setState(FALSE);
@@ -843,7 +843,7 @@
if (this->logFile)
{
fclose(this->logFile);
-   delete this->logFileName;
+   delete[] this->logFileName;
}
this->logFile = fopen(file, "w");
if (this->logFile == NULL)
@@ -1078,7 +1078,7 @@
this->inst = inst;
};
~EdInfo () {
-   if (this->nodeName) delete this->nodeName;
+   if (this->nodeName) delete[] this->nodeName;
};
 };
 //
@@ -1132,7 +1132,7 @@
"Yes", "No", NULL,
XmDIALOG_FULL_APPLICATION_MODAL
);
-   delete confMsg;
+   delete[] confMsg;
questionPosted = TRUE;
}
 }

Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[David L Thompson]

These have already been rolled into 4.3.2.

David

 Original Message 
From:   [EMAIL PROTECTED]
Date:   Tue 12/9/03 6:03
To: opendx2-dev@lists.berlios.de
Subject:Re: [opendx-dev] [PATCH] delete of something allocated with 
new[]

I'll try out these patches unless someone
else wants the job.
> This patch should take care of the
> 
> "delete of something allocated with new[]"
> 
> part of
> 
> http://opendx.watson.ibm.com/dx/mailArchives/mails.html/opendx-dev.0310/msg5
> .html

Re: [opendx-dev] [PATCH] delete of something allocated with new[]

[l . planeta]

I'll try out these patches unless someone
else wants the job.
> This patch should take care of the
> 
> "delete of something allocated with new[]"
> 
> part of
> 
> http://opendx.watson.ibm.com/dx/mailArchives/mails.html/opendx-dev.0310/msg5
> .html

[opendx-dev] [PATCH] delete of something allocated with new[]

[Marco Morandini]

This patch should take care of the

"delete of something allocated with new[]"

part of

http://opendx.watson.ibm.com/dx/mailArchives/mails.html/opendx-dev.0310/msg5.html


Regards,

Marco Morandini
Index: src/uipp/base/Command.C
===
RCS file: /src/master/dx/src/uipp/base/Command.C,v
retrieving revision 1.8
diff -u -r1.8 Command.C
--- src/uipp/base/Command.C 30 Sep 2003 17:41:46 -  1.8
+++ src/uipp/base/Command.C 9 Dec 2003 10:28:53 -
@@ -93,7 +93,7 @@
for (i=0 ; iremoveAutoCmd(this);
 
-   delete cmds;
+   delete[] cmds;
 }
 
 autos = this->activateCmds.getSize();
@@ -111,7 +111,7 @@
for (i=0 ; iremoveAutoCmd(cmds[i]);
 
-   delete cmds;
+   delete[] cmds;
 }
 autos = this->deactivateCmds.getSize();
 if (autos > 0) {
@@ -128,7 +128,7 @@
for (i=0 ; iremoveAutoCmd(cmds[i]);
 
-   delete cmds;
+   delete[] cmds;
 }
 
 delete[] this->name;
Index: src/uipp/base/IBMApplication.C
===
RCS file: /src/master/dx/src/uipp/base/IBMApplication.C,v
retrieving revision 1.23
diff -u -r1.23 IBMApplication.C
--- src/uipp/base/IBMApplication.C  30 Sep 2003 17:41:46 -  1.23
+++ src/uipp/base/IBMApplication.C  9 Dec 2003 10:28:55 -
@@ -229,15 +229,15 @@
 if (this->helpWindow)
 delete this->helpWindow;
 if (this->aboutAppString)
-   delete this->aboutAppString;
+   delete[] this->aboutAppString;
 if (this->techSupportString)
-   delete this->techSupportString;
+   delete[] this->techSupportString;
 
 if (this->noWizards) {
ListIterator it(*this->noWizards);
char *nowiz;
while ( (nowiz = (char*)it.getNext()) )
-   delete nowiz;
+   delete[] nowiz;
delete this->noWizards;
this->noWizards = NUL(List*);
 }
@@ -1087,7 +1087,7 @@
 XrmPutLineResource (&db, resource_line);
 XrmPutFileDatabase (db, res_file);
 XrmDestroyDatabase(db);
-delete resource_line;
+delete[] resource_line;
 }
 
 boolean IBMApplication::isWizardWindow(const char* name)
Index: src/uipp/base/MainWindow.C
===
RCS file: /src/master/dx/src/uipp/base/MainWindow.C,v
retrieving revision 1.13
diff -u -r1.13 MainWindow.C
--- src/uipp/base/MainWindow.C  30 Sep 2003 17:41:46 -  1.13
+++ src/uipp/base/MainWindow.C  9 Dec 2003 10:28:57 -
@@ -129,13 +129,13 @@
 ASSERT(theApplication);
 theApplication->unregisterClient(this);
 
-if (this->title) delete this->title;
+if (this->title) delete[] this->title;
 
 if (this->hasMenuBar && this->commandScope)
delete this->commandScope;
 
 if (this->geometry_string)
-   delete this->geometry_string;
+   delete[] this->geometry_string;
 }
 
 //
@@ -519,7 +519,7 @@
 else if (!EqualString(this->title, name))
 {
if(this->title)
-   delete this->title;
+   delete[] this->title;
this->title = DuplicateString(name);
 } else {
titles_equal = TRUE;
@@ -539,9 +539,9 @@
this->setGeometry(x,y,w,h);
}
if (old_geom_string)
-   delete old_geom_string;
+   delete[] old_geom_string;
} else if (this->geometry_string) {
-   delete this->geometry_string;
+   delete[] this->geometry_string;
this->geometry_string = NUL(char*);
}
 } 
Index: src/uipp/base/Strings.C
===
RCS file: /src/master/dx/src/uipp/base/Strings.C,v
retrieving revision 1.14
diff -u -r1.14 Strings.C
--- src/uipp/base/Strings.C 30 Sep 2003 17:41:46 -  1.14
+++ src/uipp/base/Strings.C 9 Dec 2003 10:28:57 -
@@ -127,15 +127,15 @@
unique = new char[strlen(path) + 12];
strcpy(unique, path);
strcat(unique,"/tmpXX");
-   delete path;
+   delete[] path;
 } else {
if (path)
-   delete path;
+   delete[] path;
unique = DuplicateString("tmpXX");
 }
 
 if (!mktemp(unique)) {
-   delete unique;  
+   delete[] unique;
unique = NULL;
 }
 
Index: src/uipp/base/UIComponent.C
===
RCS file: /src/master/dx/src/uipp/base/UIComponent.C,v
retrieving revision 1.10
diff -u -r1.10 UIComponent.C
--- src/uipp/base/UIComponent.C 30 Sep 2003 17:41:47 -  1.10
+++ src/uipp/base/UIComponent.C 9 Dec 2003 10:28:58 -
@@ -132,10 +132,10 @@
delete[] this->name;
 
 if (this->help_msg)
-   delete this->help_msg;
+   delete[] this->help_msg;
 
 if (this->inactive_help_msg)
-   delete this->inactive_help_msg;
+   delete[] this->inactive_help_msg;
 }
 
 void UIComponent::clearRootWidget()
@