This is the mail archive of the cygwin@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Re: BUG - Cygwin to GNU CC compatibility


I think Arash Partow wrote:
> i doubt the efforts of the cygwin group can surpass those of borland
> and GCC and now after recieving your mail visual c++
> It seems that all these systems can produce perfectly running
> executables, running both on linux, solaris and windows.
> 
> I don't think there is any luck involved in this, there is a bug and it
> needs to be fixed. out of the 4 compilers i compiled this code on only
> cygwin was the one to fail.
> 

This isn't a case where what we "think" matters a whole lot. The C
standard is very clear on what happens if you free a pointer twice:
undefined behavior. "Undefined" includes both "working correctly" and
"crashing" as possibilities. Even if your program doesn't crash on
some other platforms, it's wrong and must be fixed. People actually
pay good money for test-mode memory managers that crash every time you
do something illegal -- so as I said, you should feel lucky that
cygwin is offering you this learnign experience. I'm going to try
harder to explain what's going on in the rest of this message.


> >which just invokes, implicitly, the default copy constructor. This
> >default copy constructor would look something like this:
> >
> >DigitList::DigitList(DigitList &diglst) {
> >     this.digitList = diglist.digitList; // *** This line
> >     this.listSize = diglist.listSize;
> >}
> 
> when you do the copy constructor the whole object and its variable
> contents is copied automaically no need for any of the stuff you
> just wrote.

"Automatically" as in "by magic?" No, "automatically" as in "by
invoking a default operator," which has the given semantics. Another
correspondent actually pointed out, correctly, that I made a mistake:
the default copy constructor is not invoked, it's the default
assignment operator -- but that function actually looks pretty much
the same. They both do memberwise assignment.

> >
> >The line I've marked with a *** is the trouble maker -- it makes an
> >alias for the pointer member digitList (which is effectively an
> >int*). When you copy a DigitList using this copy ctor, you get two
> >DigitLists sharing a single digitList pointer.
> >
> The program does not any any way use any variable more than once,


Ah, but there's where you're wrong. Look at your constructor for
CounterBlock: 

CounterBlock::CounterBlock(DigitList digitlist) {   
   digitList.loadDigitList(digitlist);   
   numberOfDigits  = digitList.getDigitListSize();
   currentPosition = 0;
}

It accepts a DigitList *by value*, which means the DigitList you pass
as an argument is copied when this function is called. The copied
DigitList is destructed at the end of this constructor. Because the
DigitList copy constructor makes a copy of its pointer member
digitList (this is why I showed you what the default copy ctor and
assignment operators look like) that means that the original object
that was passed in as an argument now has a previously freed pointer
for a member. In countertest.cpp, you use DigitList like this:

  CounterBlock counterblock0(DigitList(dig1,3))

i.e., you pass a temporary DigitList as an argument to the
CounterBlock constructor shown above. Now, that temporary immediately
gets copied, then the copy is destructed, and then the temporary is
destructed. Boom, you've freed a pointer twice.

Again, a timeline:
        in main: DigitList#1 constructed.
        on entry to CounterBlock::CounterBlock: DigitList#1 copied to #2.
        on exit from CounterBlock::CounterBlock: DigitList#2 destructed.
        in main: DigitList#1 destructed.

Because #1 and #2 share a pointer to  the same int array, that int
array has been freed twice, and your program is allowed to crash.

> 
> >BTW, 1) why are you using free and malloc instead of new and
> >delete, anyway?
> if you actually looked at the code, malloc was used for the
> initialisation of a structure not a class, i don't know where you
> come from but on earth we use malloc to initialise and structure
> on memory according to its size.

Well, in C++, as opposed to on earth, I suppose, you're encouraged to
use new and delete for all memory management in preference to malloc
and free. Note that in C++, a struct and a class are exactly the same
thing; the only difference is that a struct's members are public by
default.

> 
> 
> >and 2) Why are you writing a class like DigitList in the
> >first place -- why not simply use a vector<int>? and
> becuase its my prerogative to write such a class, i do not
> want to use the stl.

That's a shame, because if you replaced all your int*'s with
vector<int>'s, you wouldn't be having any of these problems.

> until either a patch comes along or someone can explain why it works
> on every other os and compiler except for when its compiled by cygwin.

I hope I've explained this more clearly this time. Once you've been
programming for a year or so, you'll come to understand this kind of
issue. 

You wouldn't know Paul Derbyshire, would you?

---------------------------------------------------------
Ernest Friedman-Hill  
Distributed Systems Research        Phone: (925) 294-2154
Sandia National Labs                FAX:   (925) 294-2234
Org. 8920, MS 9012                  ejfried@ca.sandia.gov
PO Box 969                  http://herzberg.ca.sandia.gov
Livermore, CA 94550

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]