|
Hi Mike,
1. About CWin32Error and DWORD:
* CWin32Error-defined DWORD conversion adequately provides for all DWORD usages, including the registry API that has been mentioned on this article a couple of times. The usage is so simple I actually left out discussing it in the main article.
I wonder how people miss it except may be by deliberate overlooking. If that, neither argument nor better C++ code is going to help, anyway. Barring that, here's an example:
Example
CWin32Error e = ::SomeRegAPI();
Proceed to use "e" the way you would have used dwRetVal.
if( e != ERROR_SUCCESS )
if( e == ERROR_MORE_DATA )
etc.
What permits this? The fact that ERROR_SUCCESS and ERROR_MORE_DATA and all the other ret vals are all basically defined in the same file: WinError.h (or Error.h). Irrespective of whether they set thread variable or not.
* int, long, DWORD... Gosh! I am talking Win32 here! Neither Win3.1 nor IA64!
* You can use CWin32Error to alter status logic. Obvious. If not, pl. let me know.
2. The thing preceding shock therapy ( )
* No, they seriously entertain the idea that in big multi-location, multi-teams projects, it's easier to fix the source of error, blame, not to mention bugs, if the source-code itself carried the ability to show status... If the feature is built right down to the class-level granularity. But at no appreciable extra cost. They stand to benefit by using a class like CWin32Error. Whether they will choose to use it or not takes us to a completely different realm.
3. Parsing-related apprehensions:
* Parsing is not necessary.
* Think again about app-private FACILITY. It's there. People simply don't use it (Just like MC-dlls. People don't even know about it, except as in localization issues).
4. Pattern:
Thanks for the tip. I seem only dimly aware of the book. I read Gang of Four (also have the CD). But not the one you mention.
I have virtually lost all my enthusiasm about both patterns and UML--for that matter, anything to do with large-scale architectural thinking. The trouble is, I am a contract software engineer, and people don't always (i) use patterns or (ii) incorporate a contractor's suggestion when it impinges at the architectural level. I do get a lot of freedom at the class implementation level, but should not expect more. So, through first hand experience I have learnt how to operate at sub-pattern level. This being America, I anticipate this thing to go on until my residency status changes, or people change, whichever is earlier, if possible! (Yep, there seems to be a lot of parsing going on in here!)
So, for the time being, if you would like to see external polymorphism applied to CWin32Error, I am afraid, you will have to present pseudo-code/code-fragments that show how to do it.
(BTW, which geographical area are you in? How's the market for contractors currently in there?)
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
Relax I'm not deliberately overlooking the DWORD conversion, in fact I'm not overlooking it at all. You where talking about how the status value and text where "artificially separate." This lead me to believe you where trying to model ( in a C++ object sense of the word ) a Win32 status value. I was trying to show that from a conceptual stand point Win32 doesn't have a single clean error reporting paradigm.
Again, with regards to 'status logic' and status reporting I'm trying to touch upon how the separation of the two concepts in the Win32 API could affect how you develop a C++ object model. Here's a more concrete example of what I mean. Pardon the length..
#define HIDDEN_COPY(classname)\
classname(const classname&);\
classname& operator=(const classname&)
class CWin32Error
{
public:
CWin32Error():
m_error(::GetLastError())
{}
CWin32Error(const DWORD e):
m_error(e)
{}
CWin32Error& operator=(const DWORD r)
{ m_error = r; return *this; }
operator const DWORD&() const
{ return m_error; }
protected:
DWORD m_error;
private:
HIDDEN_COPY(CWin32Error);
};
class CWinSockError : public CWin32Error
{
public:
CWinSockError():
CWin32Error((DWORD)::WSAGetLastError())
{}
CWinSockError(const int e):
CWin32Error((DWORD)e)
{}
CWinSockError& operator=(const int r)
{ m_error = (DWORD)r; return *this; }
operator const int&() const
{ return (int&)m_error; }
bool operator!() const
{ return SOCKET_ERROR == (int)m_error; }
private:
HIDDEN_COPY(CWinSockError);
};
namespace foo
{
enum tError
{
FOO_ERROR,
BAR_ERROR,
OK
};
class CFooError : public CWin32Error
{
public:
CFooError():
CWin32Error((DWORD)OK)
{}
CFooError(const tError e):
CWin32Error((DWORD)e)
{}
CFooError& operator=(const tError e)
{ m_error = (DWORD)e; return *this; }
bool operator!() const
{ return OK != (tError)m_error; }
LPCTSTR Msg() const
{ return s_fooMsg[m_error]; }
private:
static const char* s_fooMsg[];
private:
HIDDEN_COPY(CFooError);
};
__declspec(selectany) const char* CFooError::s_fooMsg[] =
{
"Foo error!",
"Bar error!",
"Error success",
};
};
class CErrorFormater
{
public:
static int FormatError(const CWin32Error& e, TCHAR* pBuf, const int len)
{
if(!pBuf || !len)
return -1;
TCHAR* pTemp = NULL;
int nLen = ::FormatMessage(
FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_IGNORE_INSERTS |
FORMAT_MESSAGE_FROM_SYSTEM,
NULL,
e,
MAKELANGID( LANG_NEUTRAL, SUBLANG_DEFAULT ),
(LPTSTR)&pTemp,
1,
NULL );
_tcsncpy(pBuf,pTemp,len);
::LocalFree( pTemp );
pTemp = NULL;
return 0;
}
static int FormatError(const foo::CFooError& e, TCHAR* pBuf, const int len)
{
if(!pBuf || !len)
return -1;
_tcsncpy(pBuf,e.Msg(),len);
return 0;
}
};
class CErrorMsg
{
public:
CErrorMsg(CWin32Error& e)
{
if(-1 == CErrorFormater::FormatError(e,m_msg,sizeof(m_msg)/sizeof(m_msg[0])))
{
_stprintf(m_msg,_T("Failed to format message, for win32 error = %d"),e);
}
}
CErrorMsg(CWinSockError& e)
{
if(-1 == CErrorFormater::FormatError(e,m_msg,sizeof(m_msg)/sizeof(m_msg[0])))
{
_stprintf(m_msg,_T("Failed to format message, for win32 error = %d"),e);
}
}
CErrorMsg(foo::CFooError& e)
{
if(-1 == CErrorFormater::FormatError(e,m_msg,sizeof(m_msg)/sizeof(m_msg[0])))
{
_stprintf(m_msg,_T("Failed to format message, for foo error = %d"),e);
}
}
operator LPCTSTR() const
{ return m_msg; }
private:
TCHAR m_msg[256];
private:
HIDDEN_COPY(CErrorMsg);
};
Note I didn't spend any time thinking about this so I'm sure the design has tons of short commings. It's intended purely to spur some discussion about design issues. Namely that because the Win32 error concept is fragmented spliting the status value from the status description might offer some benifits.
CWin32Error e = ERROR_FILE_NOT_FOUND;
if(e != ERROR_SUCCESS)
{
std::cout << (const char*)CErrorMsg(e) << std::endl;
}
::WSASetLastError(WSAECONNABORTED);
CWinSockError w = SOCKET_ERROR;
if(!w)
{
std::cout << (const char*)CErrorMsg(CWinSockError()) << std::endl;
}
foo::CFooError f = foo::FOO_ERROR;
if(!f)
{
std::cout << (const char*)CErrorMsg(f) << std::endl;
}
With the status value split away from any reporting functionality the value class is now just a tight wrapper around a DWORD. So it's only a DWORD in size.
Also it's now very narrow in focus allowing a hierarchy to be developed off it that can be used to add status specific functionality - checking for success or failure of known derived types.
Splitting the error formating out from the status value allows for different types of formating specific to the different types of errors.
And finally splitting the buffering out of the formating class allows for different buffering strategies.
|
|
|
|
|
Hi Mark,
1. About relaxation.... As of today, I am. (Pun intended.)
2. About the code you have taken the effort to supply. Thanks, and yes, it's a beauty in many respects! I do disagree at many places (see point 3), but the overall cluster of classes scores too many good points to let it go unappreciated.
3. Where I differ: I think msessages still should be tied in integrally with DWORD as CWin32Error now does. (Cost won't be a consideration once the due update for normal processing arives.)
There exists a 1:1 correspondence between error DWORDs and error messages and that's why, in my view, it need not be modelled by separate classes. Separate classes are called for if the mapping were Many:1 or 1:Many.
Therefore, whenever I come to do the cluster, I will go ahead with this view of keeping DWORD and message buffer together.
4. Where I agree:
* A separate derived class for each of the major error types. More generally, a derived class for app-private facility types, and a separate namespaces for the app-private codes.
* A separate formatter class. That's a very neat solution to take away the complexity of MC dlls, variable argument lengths, inserts and other options.
5. So, our difference, in essential terms:
=========================================
* You say (via the concrete example):
-------------------------------------
Client uses CErrorMsg objects, CWin32Error objects and CWin32Error-derived objects. CErrorMsg uses CErrorFormatter to have the messages formatted. CErrorFormatter in turn uses the Win32 API function ::FormatMessage(), an externally allocated buffer, and the CWin32Error-derived objects to supply the formatted messages.
For Win32 API error messages proper, the client is responsible for supplying the error buffer. If he chooses to use heap, he is responsible for freeing the memory every time he checks on a message. In using the heap, he is only spared writing FormatMessage call (for which there can be a macro).
To avoid hitting the heap, the buffer can be of a fixed size irrespective of the message sizes and character set sizes (UNICODE vs ANSI). The definite increases in the memory footprint of the program, and the time-cost of repeated copies into buffers is acceptable.
The cost-reduction is attempted by separating the DWORD and these client/locally allocated buffers. An increase in the mental-thinking space due to a class in addition to the basic API DWORD is acceptable.
In normal execution, under this scheme, the program will neither format nor make a single message copy. It will also not make a single auto allocation.
There exists an essential difference between formatting CWin32Errors in the global namespace and in app-specific namespaces. For CWin32Error-derived CFooErrors (in app-specific namespaces), the method concretely shown is for the set of error messages to have static allocation (in the program's DATA section, as shown), and for each CFooError object to carry its own additional local or auto allocation.
Thus, the space cost of the private message strings themselves will be carried right in the program binary.
For Win32 API errors proper, however, there will be local/auto allocation in addition to the Win32 error-messages resource dll mapped into the process space at runtime.
* My design (borrowing some of your terms tentatively) would go:
---------------------------------------------------------------
Client uses CWin32Error or CWin32Error-derived objects, but no other objects, and no other buffer. If he wants to make specific copies of message strings, he is responsible for allocating and freeing the message copies.
CWin32Error, borrowing your terms, will use CErrorFormatter to have the messages formatted. CErrorFormatter, uses the CWin32Error-derived objects and ::FormatMessage API to format the messages. CErrorFormatter uses the buffer of CWin32Error.
CWin32Error (after the imminent update right this week or so) will not at all allocate heap in the normal (error-free) execution.
CWin32Error does not make any local or auto allocation of string buffers. Its local as well as copying size is and will be DWORD.
Under error conditions, an original CWin32Error will allocate one error message string on the heap, and it will be responsible for freeing it. There will be no local/auto allocation for message strings at all. All the copies of the original object will share the same message buffer thereby minimizing the heap allocation size too.
CWin32Error (after this week's update), for its internal maintenance, will introduce a few (less than five) error message strings via static allocation. Thus the memory footprint of the app will carry an extra 300 bytes (or less) due to its reliance on CWin32Error.
In future (when a separate CErrorFormatter arrives for CWin32Error), for app-private message strings, the suggested method--via ReadMe.txt of CWin32Error--is to use MC-compiled resource-dll. The said dll will keep the program binary footprint completely free of error messages. In normal execution, it will not even be mapped into the process space.
CWin32Error will continue to carry the ability to provide strings on demand. It will continue to supply the convenience of const char* conversion. It will continue to supply the DWORD conversion.
3. Finally, about avoiding the heap:
===================================
STL list, not to mention map, uses heap. Even if someone's kernel mode device driver implemented its own list, it would still be hitting heap--irrespective of his contiguous array implementation of a list--if the list is to be of indefinite size. Why, even ::FormatMessage() would hit the heap if you do not want to prescribe a max length for message buffers.
So come to think of it, is heap really a biggy? If poor practice shows us the reasons to avoid it, what about the following?
char sz[ 64 ];
::strcpy( sz, SomeMsgOf126BytesBecauseItHappenedToComeInUnicode );
or
#define MY_MAX_SIZE (48)
char s[ MY_MAX_SIZE ];
memset( s, 0, MY_MAX_SIZE );
strncpy( s, SomeMessage, MY_MAX_SIZE-1 );
s[ MY_MAX_SIZE-1 ] = NULL;
And then the end user sees a message dialog (done up nicely, complete with an apologizing AVI clip running on top):
"Your email could not be sent to somebody becaus"
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
Hi Ajit,
-- Where I differ: I think msessages still should be tied in integrally with DWORD as CWin32Error now does --
I'm not trying to say there is some fundemental problem with this or that it's incorrect to do this. In fact there's a lot to be said for using as few classes as possible. Too many small classes not doing enough is no better than one big class doing too much.
-- There exists a 1:1 correspondence between error DWORDs and error messages and that's why, in my view, it need not be modelled by separate classes. Separate classes are called for if the mapping were Many:1 or 1:Many. --
Two points. The first is that multiplicity of the relationship between two application domain concepts is only one component that goes into determining how to model them with classes.
Second there's only a 1:1 correspondence between a status value and an error message for a sub-set of what your trying to model. Throw in COM errors and it's 1 -> 0:1. Throw in user messages and it might very well be N -> 1.
Having said that, there is still nothing that necessarily makes it desirable to split the value from the message as I did in my example. And, again, modeling it with a single class - even if it turns into many-to-one relationship between some user errors and messages - is still a perfectly viable way to do it.
One other reason I liked the idea of splitting the value and the message was that it separates something that is used frequently from something that is used infrequently. The error value classes will likely be used in every module while the error messageing classes would likely be used only in the module resposible for reporting/logging errors.
-- Thus, the space cost of the private message strings themselves will be carried right in the program binary. --
Don't read to much into the example. The point was only that custom messages might require some type of specialized processing.
-- So come to think of it, is heap really a biggy? --
In many situations absolutely not. Especially a desk top app that's likely single threaded. But for some types of applications yes, it's very important. The heap is a serialization point. Even third party heaps like smart heap don't completely elminate the problem. For multi-threaded applications where performance is important how the heap is used is critical.
For me, this is 90 percent of what I work on so I could very well be paying too much attention to it. But that's what led me to split the buffering out of the formatter - once it was separate from the value class.
-- If poor practice shows us the reasons to avoid it, what about the following? --
Your code shows a strcpy into too small of a buffer but I'm not sure what your point is. Do you mean there might be times when the error message is bigger than the supplied buffer?
Obviously you'd never strcpy a const char* of unknown length. So given that you've protected yourself against program error if the message is too big it becomes a question of will this screw up your error reporting. Of course it will if all your messages are longer than your buffer but how likely is that? You have quite a bit of control over any custom error messages and a modest buffer will handle all the system messages.
Oh and you certainly converted your unicode message back to ansii before using strcpy on it eh?
-- // BTW, did you appreciate parentheses around 48?
-- // Nice alignment at DWORD boundaries. s not sz.
You got me here. What do the parens do for you? Why isn't the char sz[64] dword aligned?
Also, what did you mean by the second code snippet?
Btw, the compiler will memset the buffer for you:
char s[MY_MAX_SIZE] = {0};
But it's more efficient to skip initializing it (or using memset) and just zero the last byte. That can be done either before or after strncpy since your passing in sizeof(s)-1 as the max length.
Look forward to the next version of your class..
regards
mike
|
|
|
|
|
Hi Mike,
-- And am I glad you see no fundamental problem with keeping DWORD and char* together
-- About the 1:1 mapping being a subset of all possible values. Yes. I do agree that it is a subset. (For the remainder subset, CWin32Error behaves gracefully.)
But within the subset where there are values, I always thought COM had 1:1. Do you have any salient examples that you can think of off-hand for 1 -> 0:1?
About user-messages: So long as they use ::GetLastError() and ::SetLastError(), their mapping had better be (i) 1:1, and (ii) not intruding into the set of predefined Win32 errors. Or else, I do not know how they could debug anything but the simplest "hello world" programs. (But let's not blame MS folks for this. Ensuring nonintrusiveness is really simple--all you do is define your own FACILITY bits.)
-- Frequency of usage as a clue into modelling concepts. Yes. I can see your point. In this context, this being a single, tiny and efficient class, all I have to say would really be a repetition--that there is no/only negligibly small additional cost; that at runtime you pay as you go, so that frequency becomes irrelevant. But yes, from modelling angle, it would be useful to think of that parameter.
-- About heap. Heap being a serializer for ultra-fast systems. Excellent point there.
A single-class "vendor" like me, however, simply cannot afford to think beyond a certain point. I did think about it more, because of the way you had put it in an earlier message--"different allocation strategies." Hmmm... I then just convinced myself that for this one-class effort, if they find the built-in new operator inefficient, they will have far many other, serious issues to look into. Things like CreateHeap; reserving, marking pages; page-faults; assumptions about m/c RAM and disk-access speeds... just gets ever deeper and wider. At any rate, it would be very hard to imagine how only CWin32Error but no other other structs/classes would come to face these new-operator related heap problems. Whatever they do to improve the new operator, any new new-handler scheme they install, CWin32Error will benefit from it. So, there. At any rate, I can't prevent ::FormatMessage from using the slower ::LocalAlloc, and that's where the bottleneck is. There is nothing more to be done to reduce *those* calls... unless an appwide mapping scheme is to be developed--which will be beyond the scope of a tiny single class.
-- Finally, about my C-style example. Oh, don't take it very seriously; it wasn't meant that way. Sometimes life just picks up a boring kind of seriousness out of nowhere (sic) and as such times it is useful to poke a little fun here and there... by deliberately blowing things out of proportion... A bit like parsing and shock therapy you had mentioned
-- Alright. Let me try to get going with that guy's update again! I mean CWin32Error! (Sometimes working on the same source-code file, in front of that same stupid monitor, gets so boring!)
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
Hats Off for you and Briant Hart for the Superb Analysis
May be sometime if you add an article as to how you did the analysis, it would be a reall asset to all of us
Cheers
|
|
|
|
|
Minor correction. GetLastError typically returns a positive Win32 error code such as 2 for file not found. _com_error expects the error to be formated as an HRESULT.
In your above example, you should convert dwLastError to HRESULT using the macro:
HRESULT_FROM_WIN32. Example:
AfxMessageBox(_com_error(HRESULT_FROM_WIN32(dwLastError)).ErrorMessage(),MB_ICONSTOP);
|
|
|
|
|
To see description of the last error while debugging, you can just enter "err, hr" in Watch window.
|
|
|
|
|
Oleg:
Thanks for the IDE tip.
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|