on getting IOleCommandTarget wrong (and a bit in the middle about ActiveX controls)

IOleCommandTarget is very useful.  It provides a generic way of sending commands between objects.  IE makes extensive use of IOleCommandTarget, both publically and internally.  And, like IUnknown, people frequently get it wrong.

Each command is composed of a GUID (Command Group Identifier) and a DWORD (Command Identifier). 

First, what is wrong with this code:

HRESULT CFoo::Exec(const GUID *pguidCmdGroup, DWORD nCmdID, DWORD nCmdexecopt, VARIANTARG *pvarargIn, VARIANTARG *pvarargOut)
{
if (IsEqualGUID(CGID_FooCommands, *pguidCmdGroup))
{
...

The answer is obvious: pguidCmdGroup might be NULL and you crash.  Furthermore, pguidCmdGroup is frequently NULL, since NULL is how you specify the standard group. 

You might say to yourself, "Self, I don't need to worry about NULL, since I am the only consumer of this particular implementation,"  and that may be the case.  However, if you can be CoCreate()'ed, than it is definitively not the case.  Remember, any control on your machine can be instantiated as an ActiveX control.  During the ActiveX control instantiation process, mshtml will query your object for certain well-known interfaces, like IObjectSafety, IOleObject, and, you guessed it, IOleCommandTarget.  If your control responds to IOleCommandTarget, mshtml will attempt to Exec() standard commands.  If you do not guard for NULL, you will cause Internet Explorer (and other hosts) to crash.

The second set of common errors come in the form of returning incorrect error codes.  The documentation outlines the rules and you must be careful to implement them.  If you recognize the group but not the command, OLECMDERR_E_NOTSUPPORTED is the correct return value.  If you do not recognize the group, OLECMDERR_E_UNKNOWNGROUP is correct.  There is a special case though, which is when the group is NULL and the command is not supported; in this case the correct return value is OLECMDERR_E_NOTSUPPORTED.