|
For those of you who have adopted one of the various techniques that have been proposed to avoid inadvertantly calling a virtual method from a constructor, which method did you choose?
So far, this one [^] seems the most promising to me but I've not tripped over this problem (that I'm aware of  ) till today so I have not given it much thought in the past and I need some guidance.
|
|
|
|
|
If you use test driven development and write unit tests for your code you pick up very quickly when you've committed a virtual crime in a constructor or destructor. Another good way is never inheriting concrete classes but in existing code bases you've often not got that choice (and if you're using something like MFC you're well stuck).
Cheers,
Ash
|
|
|
|
|
The indicated method is a good one.
However, it's not "illegal" or "invalid" to call a virtual method in a ctor so long as you are aware of what you're doing and that's what you intend. You just need to remember what it will actually call in that instance.
"If your actions inspire others to dream more, learn more, do more and become more, you are a leader." - John Quincy Adams
|
|
|
|
|
Looks like a malposed attempt to resolve a malposed problem (so it is a good question!).
Actually calling a vistual function from a constructor is LEGAL. It is not something the dumb compiler allows even if shouldn't.
The problem is that C++ is a "complex language" and that what such a call will do may be not what dumb programmers suppose it to do.
But whoever has a minimum of logic should understand that -during the execution of a base constructor- the derived component is not yet come into existence, and hence the base v-table cannot contain references to it. The virtual function at that point is not yet been overridden.
Now trying to find a way to "fix the language" to disallow this is like trying to make C++ to turn into something else. It is not something related to your implementation. Every C++ programmer MUST KNOW what happens if a constructor / destructor calls a virtual function. Ther is nothing to fix, here.
Unless you don't like how C++ works. But -at that point- just use another language.
Afer all, if you do
#define begin {
#define end }
you'd better to use Pascal/Delphi/Ada, and not C/C++/C#/Java/D ...
2 bugs found.
> recompile ...
65534 bugs found.
|
|
|
|
|
Oh, I am aware that it is legal and the C++ behavior makes sense but I unfortunately tripped over it anyway. I wasn't criticizing the language as I still consider C++ my language of choice. I was merely looking for an approach to initialization that would protect me from inadvertantly calling some virtual function from a constructor as it is so tempting with some class hierarchies. The effect of doing so blindly had subtle effects that had me chasing my tail for a bit, quite embarrassing but a reality with so many rules to remember.
Looking for a pattern to use that will allow me to initialize the base and derived class objects, with any function (virtual or not) which requires avoiding calling methods from the constructor directly except for simple member initialization lists and such.
I'm definitely not trying to fix the language. 
thanks for the comments.
|
|
|
|
|
Hi everybody,
I have some problems reading the COM ports. In particular I have used WaitForMultipleObjects and ReadFile, that permit me
listen the COM port selected.
I use this code for reading the COM port while I was trasfering a file with Bluetooth from the phone to the computer.
I can open the right COM port (they are 4, one for the input, one for the output and two for some remote control, i think)
but in any of them I can see nothing in the buffer, because everytime the ::WaitForMultipleObjects switch
in the default case, exiting from the cycle.
My question is:
Is this the correct way to read the bluetooth packets arriving in the COM ports?
If, yes, why during the data transfert is like if no package where transfering in the port?
I have used also CSerial class present in this site with the same result.
Thank,
Anne
//[...]
HANDLE hevtOverlapped = ::CreateEvent(0,TRUE,FALSE,0);;
if (hevtOverlapped == 0)
{
cout << "Unable to create manual-reset event for overlapped I/O.\n";
return -1;
}
// Setup the overlapped structure
OVERLAPPED ov = {0};
ov.hEvent = hevtOverlapped;
// Open the "STOP" handle
string name1 = "Overlapped_Stop_Event";
HANDLE hevtStop = ::CreateEvent(0,TRUE,FALSE, name1.c_str());
// Keep reading data, until an EOF (CTRL-Z) has been received
bool fContinue = true;
do
{
// Wait for an event
lLastError = serial.WaitEvent(&ov);
if (lLastError != ERROR_SUCCESS)
{
cout << "Unable to wait for a COM-port event.\n";
return -1;
}
// Setup array of handles in which we are interested
HANDLE ahWait[2];
ahWait[0] = hevtOverlapped;
ahWait[1] = hevtStop;
// Wait until something happens
switch (::WaitForMultipleObjects(sizeof(ahWait)/sizeof(*ahWait),ahWait,FALSE,500))
{
case WAIT_OBJECT_0:
{
// Read data from the COM-port (ReadFile)
[...]
}
break;
case WAIT_OBJECT_0+1:
{
// Set the continue bit to false, so we'll exit
fContinue = false;
}
break;
default:
{
// Something went wrong
// exit
[...]
}
break;
}
}
while (fContinue);
//[...]
|
|
|
|
|
You should check better the WaitForMultipleObjects return value and, if it is WAIT_FAILED then call GetLastError, as stated in the documentation[^].
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
Firstly, please put all your code in <pre></pre> tags, or select it and click the code block button above the message content text box. This will make it easier for everyone to read.
0.5s isn't a very long time to wait for something like bluetooth, try waiting for longer.
Make sure the hevtStop isn't set
Add cases to the WaitForMultipleObjects switch for WAIT_FAILED and WAIT_TIMEOUT, in WAIT_FAILED add a call to GetLastError(), if it is hit, find the error code in the System Error Codes[^] page on MSDN
If you are using ReadFile, I wouldn't bother with overlapped operations at this stage. Either get it working then put them in, or put this code into a different thread
|
|
|
|
|
Hi!
Thank you for the replay.
- I set 4000 instead 500 with the same result (I also set INFINITE, and the program is blocked in waiting)
- I haven't done any other operation with hevtStop, it shouldn't be set
- I add WAIT_TIMEOUT and WAIT_FAILED, and it enter in WAIT_TIMEOUT, after 4 seconds.
I use ReadFile, but I use it inside WAIT_OBJECT_0, where it doesn't enter.
Sorry for the tags, in future i'll use
|
|
|
|
|
Well that really only leaves serial.WaitEvent(&ov). How and where is the event set?
Have you tried opening the COM port in something like HyperTerminal or PuTTY?
|
|
|
|
|
I use the class CSerial (founded in this site) and serial is an object of the class.
serial.WaitEvent(&ov) is this:
<pre>
LONG CSerial::WaitEvent (LPOVERLAPPED lpOverlapped, DWORD dwTimeout)
{
// Check if time-outs are supported
CheckRequirements(lpOverlapped,dwTimeout);
// Reset error state
m_lLastError = ERROR_SUCCESS;
// Check if the device is open
if (m_hFile == 0)
{
// Set the internal error code
m_lLastError = ERROR_INVALID_HANDLE;
// Issue an error and quit
_RPTF0(_CRT_WARN,"CSerial::WaitEvent - Device is not opened\n");
return m_lLastError;
}
</pre>
I haven't used HyperTerminal and Putty, but I think is a good idea for seeing the effective data tranfer (if I undestand correctly the usage)
Anne
|
|
|
|
|
Your code looks pretty much the same as the sample code, so all I can suggest for now is that you open the COM port in PuTTY or HyperTerminal and make sure that you have the right settings for the connection
|
|
|
|
|
Hi,
I try to use PuTTY and I never can see the data flowing (PuTTY says "Unable to Open the port").
I try to use Portmon (Sysinternal) but also in this case the "Device is in use" and I can't open it)
At this point i would like to try the code on some other device, if you have any ideas how can I try it (not only bluetooth, also a device using COM port, or also a simulator of data transfer in COM port)
Any idea is wellcome,
Anne
|
|
|
|
|
This is a good indication that your error handling isn't very good.
Make sure you are checking the return value of all functions that you call in CSerial.
I dont know of any virtual COM ports, but a good place to start would be google[^].
|
|
|
|
|
Hi All,
Both _stat() and _fstat() are used to obtain the opened file information._stat() does not require FileDescriptor whereas _fstat() needs FileDescriptor to obtain the file Info.
Is any other differences between these?In what scenario these can be used?
Can any one clarify my doubt?
Thanx.
|
|
|
|
|
Reffer to MSDN for function documentation.
_stat[^]
_fstat[^]
_fstat gets information based on a file descriptor (acquired with _open or fopen)
_stat gets information based on a file path
|
|
|
|
|
Can you tell me if I use in corect way these classes , CTypedPtrArray & CStringArray ?
These are member variable to CMyDoc class :
typedef CTypedPtrArray<CObArray,CStringArray*> CTableArray;
class CMyDoc : public CDocument
{
public:
CStringArray m_saTable;
CTableArray m_saTables;
...
}
and here is code snipped for use of them :
m_saTable.Add("String Test");
m_saTables.Add(&m_saTable);
CStringArray* saTest = m_saTables.GetAt(0);
CString sTest = saTest->GetAt(0);
TRACE("\n %s \n",sTest);
but , where I need to delete elements ?
I try to delete items in OnCloseDocument(...)
void CMyDoc::OnCloseDocument()
{
m_saTable.RemoveAll();
m_saTables.RemoveAll();
TRACE("\n %d - %d \n",m_saTable.GetSize(),m_saTables.GetSize());
CDocument::OnCloseDocument();
}
But with or without remove code , I didn't have memory leaks in debug window ... is really necesary to delete the items ?
Any help , I will appreciated ! Thanks !
|
|
|
|
|
Just one point to offer. Having two member variables named so similarly is likely going to lead to confusion at some later point. Try to come up with names that are more meaningful to the actual use of the data. For example if the strings in the string array class are all people names, cities of the world or street names, try to use that instead of m_saTable. Similarly with the member m_saTables. Just a suggestion. 
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
posting about Crystal Reports here is like discussing gay marriage on a catholic church’s website.[Nishant Sivakumar]
|
|
|
|
|
Thanks for the suggestion , I will fix that .
|
|
|
|
|
Firstly, make sure you are dumping the memory leaks correctly. There are a number of ways to do this, but the best is by adding
#if defined(DEBUG) || defined(_DEBUG)
_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
#endif
as early in your code as possible, in your case it will be the constructor of your App object BOOL CxxxApp::CxxxApp()
This is the only way will dump the memory leaks after all static objects have been destroyed. All other ways that I know of will give misleading results if you have static objects
If no destructor is provided for your class 1 is generated which calls the destructors of all member classes, otherwise the destructors of all member classes is called after your destructor code.
In destructors you only need to delete memory allocated by new, malloc or the likes.
Either way, your code is calling the destructors to m_saTable and m_saTables and these destructors will free any memory associated with it.
Having said that, your CTableArray is storing pointers, which means that the destructor for the table will only free the pointers, and not the actual data.
You will need to enumerate each item and free its memory manually. CMyDoc::OnCloseDocument() would be a good place to do this.
INT_PTR nElements = m_saTables.GetCount();
for (int nIndex = 0; nIndex < nElements; ++nIndex) {
delete m_saTables.GetAt(nIndex);
}
|
|
|
|
|
At this code :
#if defined(DEBUG) || defined(_DEBUG)
_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
#endif
give me the follow error :
error C2501: '_CrtSetDbgFlag' : missing storage-class or type specifiers
Because I use VC6 ?
|
|
|
|
|
Possibly.
Try #include <crtdbg.h> and see if that fixes the problem, otherwise try _CrtDumpMemoryLeaks(); as the last statement in the App destructor CxxxApp::~CxxxApp()
I havn't used VC 6 in a many years, and it may not have these debugging functions. MSDN only has version 7.1 (2003) onwards.
If I might make a suggestion, it might be an idea to upgrade.
VC++ 7.1 (2003) is the latest version to support Windows 95, VC++ 8.0 (2005) is the latest version to support Windows 98/ME, VC++ 9.0 (2008) and 10.0 (2010) support Windows 2000 onwards.
Unless you are wanting to support these older platforms I would suggest upgrading to at least VS 2008 Express. It is a free download from the Microsoft website with a Windows Live ID.
|
|
|
|
|
I don't know if is corect , but I think that I solve it :
typedef CTypedPtrArray<CObArray,CStringArray*> CDumpArray;
class CMyDoc : public CDocument
{
protected:
CMyDoc();
DECLARE_DYNCREATE(CMyDoc)
public:
CDumpArray m_arrDump;
...
};
CString sText = "one";
CStringArray* pTable = NULL;
pTable = new CStringArray();
pTable->Add(sText);
sText = "two";
pTable->Add(sText);
m_arrDump.Add(pTable);
pTable = new CStringArray();
sText = "three";
pTable->Add(sText);
sText = "four";
pTable->Add(sText);
m_arrDump.Add(pTable);
void CMyDoc::OnCloseDocument()
{
TRACE("\n m_arrDump %d \n",m_arrDump.GetSize());
CString sTemp;
for(int n = 0;n < m_arrDump.GetSize();++n)
{
TRACE("\n StringArray no %d \n",n);
CStringArray* pTable = m_arrDump.GetAt(n);
for(int t = 0;t < pTable->GetSize();++t)
{
sTemp = pTable->GetAt(t);
TRACE("\n %s \n",sTemp);
}
}
int nCount = m_arrDump.GetSize();
for(int i = 0;i < nCount;++i)delete m_arrDump.GetAt(i);
m_arrDump.RemoveAll();
TRACE("\n m_arrDump %d \n",m_arrDump.GetSize());
CDocument::OnCloseDocument();
}
|
|
|
|
|
I cant see anything wrong with this, however a better solution for CMyDoc::OnCloseDocument() would be
void CMyDoc::OnCloseDocument()
{
TRACE("\n m_arrDump %d \n",m_arrDump.GetSize());
CString sTemp;
int nCount = m_arrDump.GetSize();
for(int n = 0;n < nCount; ++n)
{
TRACE("\n StringArray no %d \n",n);
CStringArray* pTable = m_arrDump.GetAt(n);
for(int t = 0;t < pTable->GetSize();++t)
{
sTemp = pTable->GetAt(t);
TRACE("\n %s \n",sTemp);
}
delete pTable;
}
m_arrDump.RemoveAll();
CDocument::OnCloseDocument();
}
|
|
|
|
|
Thanks , works fine ! Thank you all !!!
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
