|
Thanks for this class.
|
|
|
|
|
|
doesnt make sense at all.
|
|
|
|
|
|
Ajit,
Will I be able to display error messages in different languages.
--Milind
|
|
|
|
|
Hello Ajit !
First i wan't to thank you about your article, i will try the Cwin32Error class.
Anyway,i'm writing a Dll, and instaed of writing my Error Handling, i'm using SetLastError (Win function).
When error occured,Dll function use the SetLastError function to set an error code. The calling function then can call GetLastError to retrieve the error code.
When i tried this in one .exe , it works. but when i called to a Dll the GetLastError did not retrieve the error code which i expect.
Some how, when working with Dll, it doesn't go well.
if Cwin32Error encapsulate the GetLastError() maybe it will happen there too ?
if you can help, i will be very thankfull to you !
Michael.
Michael
|
|
|
|
|
WFC's wfc_get_error_string is even simpler and faster... though I get annoyed by its prototype:
void PASCAL wfc_get_error_string( DWORD error_code, CString& error_string )
I often wrap it in another function that looks more like:
CString GetErrorString( DWORD dwErrorCode )
|
|
|
|
|
1. Your MyFunction vanilla (?) sample returns a locally declared CWin32Error. Is this ok? Not only in the terms of costs, but correctitude.
2. Do you think that calling a lot of constructors for this class is free?
3. GetLastError returns the last error code in the calling thread. So in some cases this class cannot be used at all.
4. "Most functions in the Win32 API that set the thread’s last error code value set it when they fail; a few functions set it when they succeed." (MSDN excerpt). So your class can throw an exception when everything is ok.
5. Some functions expects a last error like ERROR_MORE_DATA in order to retrieve the correct size used to allocate a buffer to pass in a second call. Just think at some registry enumerations, NetApi* or QueryService* routines. Since you cannot predict the number of items and their total size, it is quite normal that the first call (or every 'first' call in a 'do {...} while(TRUE)' loop) will fail returning this kind of error(s). So?
|
|
|
|
|
Dear Sardaukar:
I will try to be brief
Re: Your points 1. and 2: MyFunction (Vanilla version) does not return locally declared CWin32Error but its copy.
Correctitude: In C++, user-defined types ought to be treated at par with built-in types. Remember: If you return an int variable, you are still asking the compiler to produce code to copy four bytes. For CPoint : eight bytes. For const char* allocated on the heap (and not locally declared): four bytes--plus other code you are bound to write to check its lifetime elsewhere in the program to avoid runtime failures. For CWin32Error : four bytes plus just a few inlined code steps involving just one call to InterlockedIncrement(). InterlockedIncrement(), handled right in the Win32 subsystem rather than after a contex switchover to kernel mode, is very fast.
No function returns are ever free--for user-defined types or built-in types. But, overall, the cost involved in returning CWin32Error object by value, rather than an int value, carries so small an extracost that it can be even smaller than the overhead of stack-frame maintenance code introduced due to an extra function you write elsewhere to check the health of an on-the-heap or a static object.
So, there always is cost. The issue is: how do you organize it. CWin32Error urges to exploit the C++ language features and so lets you organize in the cleanest way with no extra overheads.
Re: Your concerns in 3. and 4.: That's why CWin32Error copy constructor and assignment operators do not call GetLastError(). Please see the class comments and documentation for further details.
CWin32Error guaruntees--I repeat guaruntees--never to throw an error when everything is OK. Two interpretations of this statement: (i) It never calls an extra GetLastError(). If at all necessary (which I doubt), you can always use CWin32Error::ReGetLastError member function. (ii) To be accurate, you throw CWin32Error. CWin32Error does not raise any exception on its own (which is something that some other people may not like )
Re: Your point 5: The sample usage is just that: sample. It does not cover every usage scenario. For the case you write about, the code would look something like:
char* sz = new char [64];
int nBufSize = 63;
int nRequired = -1;
if( ! SomeWin32API( sz, nBufSize, &nRequired ) )
{
delete [] sz;
sz = new char [nRequired];
if( ! SomeWin32API( sz, nBufSize, &nRequired ) )
{
delete [] sz;
CWin32Error e;
}
}
Hope this clarifies.
Bottomline: CWin32Error can be used without hesitation in all cases.
Good points, Sardurkar... Tells me you are seriously thinking of using CWin32Error--which is very welcom. Feel free to voice your other concerns about CWin32Error too.
-- Ajit Jadhav
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
Hey Guy,
Microsoft's already ahead of you there, with the Compiler COM Support's _com_error class.
It's OK to use _com_error even if you aren't writing COM software. Essentially, it takes any error you would normally call GetLastError() on, or any HRESULT from a COM call, and will give you the text equiv. of the error, all with just one call.
It works like this:
DWORD dwLastError = ::SomeSDKFunction(...);
if (dwLastError != 0)
{
AfxMessageBox(_com_error(dwLastError).ErrorMessage(),
MB_ICONSTOP);
return -1;
}
That's it. The class can be used just by #including <comerror.h> in your STDAFX.H file, and it can also be thrown in an exception.
Yours,
Brian Hart
|
|
|
|
|
If you're programming for CE, _com_error is not supported. Perhaps his class would be useful for those people.
|
|
|
|
|
CWin32Error compared with _com_error
In the following block, _com_error internally calls
::FormatMessage() four times
::LocalFree() four times
delete [] zero times
{
_com_error e1 = 0x8000000A;
_com_error e2 = 0x8000000A;
const TCHAR* szDesc1 = e1.ErrorMessage();
const TCHAR* szDesc2 = e2.ErrorMessage();
e2 = e1;
e1 = e2;
const TCHAR* szDesc3 = e1.ErrorMessage();
const TCHAR* szDesc4 = e2.ErrorMessage();
int nSize = sizeof(e1);
}
On the other hand, CWin32Error internally calls
::FormatMessage() two times
::LocalFree() two times
delete [] two times
{
CWin32Error e1 = 0x8000000A;
CWin32Error e2 = 0x8000000A;
const TCHAR* szDesc1 = e1;
const TCHAR* szDesc2 = e2;
e2 = e1;
e1 = e2;
const TCHAR* szDesc3 = e1;
const TCHAR* szDesc4 = e2;
int nSize = sizeof(e1);
}
Absent method calls, the CWin32Error code looks prettier to my eyes...
Beauty is in... Analysis
Analysis
1. Time Cost
1.1 When the above blocks are run repeatedly, CWin32Error -based code runs almost twice as fast as _com_error -based code does.
To repeat: CWin32Error is faster.
1.2. To be fair: After commenting out the szDesc2 and szDesc3 defining lines in the above test, both classes
are almost equally fast (differing by less than 1% in actual execution tests in the release build).
1.3 The real determinant of the execution time appears to be the number of times FormatMessage is called.
Algorithmic Rationale of Time Cost
2.1 CWin32Error employs:
Hungry capture of formatted strings, but
Smart copying of strings to minimize overheads.
2.2 _com_error employs (for the releavant ErrorMessage() member-function):
Lazy capture of error strings, and
Repeated capture of error strings which canmean dumb calls in certain circumstances as the above simple test shows.
3. Space Cost
sizeof(_com_error) is 16 bytes. That is almost as large
as many people's full name spelt in ASCII characters.
sizeof(CWin32Error) is 4 bytes. That is the smallest addressable variable size possible on Win32.
This makes CWin32Error objects one-fourth the size of _com_error objects.
sizeof(string buffer) in both cases, of course, is identical.
4. Conclusion of Cost Analysis
Since the time cost is similar or smaller by a constant factor for CWin32Error , and since the space-cost is decidedly smaller for CWin32Error , therefore, CWin32Error is a better choice for performance-critical applications such as function-return values and exception-handling.
5. The Road Behind
5.0 The main purpose--and hence the class design--differs.
5.1. _com_error may be best thought of as an easy programming interface to the (D)COM(+) Interface IErrorInfo . In this light, it really has been done up extremely well by any standards.
5.2 On the other hand, the goals for CWin32Error did not include support for IErrorInfo or for COM but to simply focus on providing:
the smallest,
the fastest, and
the most generally useful
possible Win32 error class with
the least source- and compile-dependencies elsewhere.
and to allow as pretty programming as possible (sending full object without method invocation to TRACE, for example)
6. An Exercise Ahead
Interested parties may perform similar analyses of
ATL CComBSTR vs
VC++ _bstr_t vs
MFC CString under UNICODE vs
STL wstring
...
And also keep us all posted here
Special thanks to Briant Hart for galvanizing me into analysis. I wouldn't have done it myself!
Ajit Jadhav
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
|
|
"Can we still be friends?"
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
First let me say I love classes like CWin32Error. Simplifying the little things that we encounter constantly is great. But there are a couple issues with how your using it.
For example given your second function:
CWin32Error MyFunction( void );
Yes your class is much more efficient than _com_error the best choice of all is still going to be:
DWORD MyFunction(void);
For the simple reason you never want to hit the heap for no reason during the normal flow of your program - and there's no reason to hit it for success.
Also, in general you never want to catch exceptions by value. Yes it'll happen when throwing/catching simple types but your class is *not* a simple type nor a replacement for one. So when catching CWin32Errors you'd still want to:
catch(CWin32Error& e)
{}
Thanks for posting your code! CWin32Error has some very nice features.
mike
|
|
|
|
|
Hi Mike,
Thanks for your thoughtful comments! I really appreciated your comment.
2. Let me reply on your second point first (ie. catching by ref). This is an issue independent of CWin32Error, and really requires a separate discussion of catching, throwing, and especially, rethrowing. Yes, I agree completely that catching by ref involves much less cost, especially as long as intermediate throw s too have passed the reference to the original object. But this is really at the discretion of the programmer, and so, a somewhat separate issue.
The sample scenario in CWin32Error is just an advertisement to show that even a novice programmer can't incur too many costs with CWin32Error--smart copying will kick in.
2. About your first point (i.e. the heap usage in normal times (no-error) at runtime). Good observation! The point is well taken.
Soon after posting, I started thinking of an upgrade to further improve runtime conditions precisely on the lines you mentioned. But I realized it only after posting the code. (That always happens!!). Please wait a few days time to see the planned upgrade.
If interested here's the descriptive preview of what it will have.
Warning: Being descriptive, it's a bit lengthy. Also very tentative. (I am not sure I really need the extra static variable I mention in the following description). Anyway, check out the CWin32Error page within a week for the upgrade.
The specific solution I am currently thinking is to introduce a class static member variable to hold the error code in normal times (error == 0). Then, there can be hungry capture of only the numeric code in construction or in ReGetLastError(). The fallout (after such upgrade) will be:
On the Plus side:
----------------
(i) For Normal Execution (member error == 0 ) at Runtime: CWin32Error will then aovid hitting the heap altogether.
Thus, CWin32Error will be costlier to calling ::GetLastError() only by an extra if condition, testing only equality of int variables. For a program that run completely normally, this would mean, (a) no new[], (b) no delete[], (c) not even a single InterlockedXXX() call introduced by CWin32Error during the entire program execution!
(ii) For Execution Under Error Conditions (member error != 0 ): An improvement I plan here is to postpone FormatMessage calls until needed. Thus, CWin32Error will hit the heap only if and only when the client code actually calls CWin32Error's operator const char** or Description() member. Of course, CWin32Error will continue to have smart copying mechanism kicked in from that point on.
On the Minus Side:
-----------------
An extra space cost of a static member variable of unsigned int type. That is, 4 static bytes (only). Being static, the space cost will be constant, irrespective of the # of CWin32Error objects at runtime.
It seems to me that the plus side far outweighs the minus side. So, please check out the CWin32Error page later this week for the above upgrade.
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
Ajit,
Ok catching exceptions is hardly what your post is about so I'll leave off any lengthy discussion of it but.. I will go out on a limb and say there is no reason to catch anything, other than a simple type, by value. It'll only lead to problems. So it's at the discretion of the programmer only because the compiler allows it.
Almost all my work related programming involves multi-threaded server applications so I cringe anytime anyone mentions statics. It takes a lot of work to make a class with read/write static variables thread safe and efficient.
More than that however just make sure you've thought out how your class is going to be used. It's tempting to envision using CWin32Error as you did in your second example:
CWin32Error MyFunction();
But is it really practical? Or even necessary? Frankly the last example you gave is where CWin32Error shines - it simplifies the code. In fact it could have become simply
if( AlertUser(dwErr))
{
AfxMessageBox(CWin32Error(dwErr));
}
Very nice!
I might even go farther by hiding the copy constructor and assignment operators. This would simplify the class even more and shrink it down to it's core functionality - which is formating error messages. And if you wanted to improve the class I'd look at things that go towards it's central function. For example you could add alternate constructors that provided formating custom error messages along with the Win32 error messages. This would let you use it something like:
int loop = 0;
// doing something in a loop that fails...
if( AlertUser(dwErr))
{
AfxMessageBox(CWin32Error("failed during %d try at doing Foo",loop,dwErr));
}
Another thought would be to provide a way to extract error messages without hitting the heap at all. Something like:
char buffer[512];
CWin32Error(buffer,sizeof(buffer));
Anyway, the point is to avoid trying to add to many bells an whistles. Think about how you'll use the class most effectively and design it to maximize that core functionality with a minimal interface. You'll end up with an effective, useful class, that you return to time and time again.
Mike
|
|
|
|
|
Hi Mike,
Thanks!
A few comments right here. More, later. Perhaps offline discussion by email??
- The core idea behind this class: CWin32Error objects as first-class entities indicating runtime status. (Perhaps the class-name is a misnomer, but I wanted to keep it associated with ::GetLastError to spread usage.)
By core idea, CWin32Error is not just a DWORD number, not just a TCHAR string, not just a conversion functionality, but an integration of them all. A status-indicating thing. I just happened to take the view that the Win32 API seems to artificially separate the two (to maintain C-compatibility); and that in C++ programming, a char* string should be as easily available as the DWORD is (without losing the convenience and the small runtime cost of the DWORD). That was the core idea. Perhaps not always possible cost-wise. But worth getting as close to DWORD usage as is possible.
So, whatever you think of doing with a DWORD, it should be possible to do the same with the object. For example, TRACEing, returning, throwing exceptions, as a member variable in the class, as true state-indicating objects (using app-private FACILITY codes) in a state-transition map, anything else....
That in turn meant making the class time-efficient in terms of FormatMessage calls it makes, and space-efficient in terms of its sizeof value and avoiding multiple string buffers as far as possible. Hence, the rest of the class code involving smart copying and meta-mem management.
But to think of a member function returning a char* given a DWORD would be somewhat off the core of the C++ idea--it will be closer to the C-way of thinking, not C++.
Actually, any DWORD => char* conversion should be seen as only a part of the class. Per core idea: you have this error/status object, and you probe it for whatever reason, and it will return what it knows about the status and/or health of the program in that context. You can be a user (returns a human readable, standard string message) or a programmer (returns a variable-address that is TRACE-compatible) or the program at run-time (captures and returns error-code numbers with negligible overheads).
- The Received Opinion varies a lot for throwing user-defined types in addition to built-in types. For example, the standard C++ includes "exception" class, and Stroustrup advocates thinking exception first. On the other hand, there can be situations where exception frames and unwinding cost and/or semantics is not most desirable. In these later cases, CWin32Error can be used in the simple object sense.
- Ditto for retvals. A lot of folks in quite a few projects I worked on seemed to require something like returning status-strings! Returning status strings seems like a natural thing to think for many programmers--and not all of them are recent converts from VB. If they can have smart-copies...
- About multi-threading and static member variables. Yes of course. Any static members, if at all introduced, will be purely readonly.
Looks like statics (a small array of them, in fact), will be inevitable under the constraints: viz. that CWin32Error will not raise exceptions on its own. Here, I am aware that memory for exception frames and objects is guarunteed by the C++ language. But many may deliberately disable exception-handling support in the code generation (e.g. ATL COM projects.)
Mem allocation, and even FormatMessage can fail internal to the class. Yet, by core-idea, the user-programmer will not check on DWORD before using the const char* conversion. Now, rather than introduce static allocation into the program from back-door via macro substitution:
if (msg string is not available) return _TEXT("class internal error message")
it would be better to introduce static readonly variables explicitly. There seems to be no other choice when memory allocation the internal buffers fail, and yet the user-programmer has fully relied on the const char** operator in his code.
- Hiding copy ctor, = operator : Interesting idea.
- Extracting into user-providing buffer. This is a v. good way of looking at the class, and might increase its practical appeal. Sure will include this functionality.
- Loop-type usage: 2B | ~2B in this case: Supporting printf-style format strings turns out to be a desirable revision but a big one. Variable sized args, preproc defines and format string interpretation (UNICODE/OLE2ANSI...), processing failure possibilities, MC-compiled message resources and their DLL hInstance, etc... Perhaps this is better served by a small cluster of classes than a single class. TBD: in future.
- About minimal i/face: Yes... I am reminded that famous quote (don't remember who said it): Interface design attains perfection not when there is nothing else left to add, but when there is nothing else left to take away.
Thanks again for your interest. Unbelievably, writing this smallest of classes turns out to be such a big-time headache! Thoughtful comments like yours help a lot! And I seem unable to refrain from writing loooong messages!
-- Ajit
Your project today is your great-grandchildrens' "ancestor clock."
|
|
|
|
|
-- By core idea, CWin32Error is not just a DWORD number, not just a TCHAR string, not just a conversion functionality, but an integration of them all. A status-indicating thing. --
But the Win32 status value really is just a DWORD. Worse than that it's actually just a DWORD *sized* value. In some instances it's an int, in some a long. And to further complicate matters it's not always associated with ::GetLastError(). The registry functions for exmaple don't set ::GetLastError() instead they directly return their error value. (as a long).
-- I just happened to take the view that the Win32 API seems to artificially separate the two (to maintain C-compatibility); --
I would argue that it's much more desirable in the C++ modeling world to separate the two simply because they do completly different things. The status value allows us to alter program logic and respond to error conditions, the explanatory text allows us to format nice error messages.
-- A lot of folks in quite a few projects I worked on seemed to require something like returning status-strings! --
I'm not sure what you mean by a status string. Do you mean they want you to return an LPCTSTR that they're going to parse to determine if the function succeeded or failed? If so just take them by the hand and lead them to the padded room for shock therapy.
-- Yet, by core-idea, the user-programmer will not check on DWORD before using the const char* conversion. --
Would you ever routinly format an error message before being certain there was an error? I can't think of a single scenario.
-- Actually, any DWORD => char* conversion should be seen as only a part of the class. Per core idea: you have this error/status object, and you probe it for whatever reason, and it will return what it knows about the status and/or health of the program in that context. You can be a user (returns a human readable, standard string message) or a programmer (returns a variable-address that is TRACE-compatible) or the program at run-time (captures and returns error-code numbers with negligible overheads). --
There's an interesting pattern - "external polymorphism" - presented in the Pattern Languages Of Program Design 3. It deals with how to extend simple types. This pattern might allow you to express a CWin32Error as a very tight wrapper around a DWORD and then add give it additional functionality using the external polymorphism pattern. Good book even if that pattern doesn't apply in this case
|
|
|
|
|
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."
|
|
|
|
|