26. 狡猾的VARIANT_BOOL
下面的代码选自 NAME 项目。代码中的错误被 PVS-Studio 诊断为:V721 VARIANT_BOOL
类使用不正确。true(VARIANT_TRUE)等于-1。检查第一个参数。
|
virtual HRESULT __stdcall
put_HandleKeyboard (VARIANT_BOOL pVal) = 0;
....
pController->put_HandleKeyboard(true);
|
解释
有一段很机智的话是这么写的:
自那些年我们学习了Basic后,我们就不得不为我们自己所犯下的错误而买单了。
这个提示就是关于邪恶的。VARIANT_BOOL 是 VB 里的一种类型。我们现在遇到的一些编程问题都跟这个类型有关。因为,它的 true 等于 -1。
让我们来看一下这个类型的声明和表示 true/false 的常数。
|
typedef short VARIANT_BOOL;
#define VARIANT_TRUE ((VARIANT_BOOL)-1)
#define VARIANT_FALSE ((VARIANT_BOOL)0)
|
看起来也没什么麻烦的。false 是 0 ,true 是非 0。所以 -1 是一个很合适的常熟。但是在使用 true 和 false 来代替 VARIANT_TRUE 的时候很容易出错。
正确代码
|
pController->put_HandleKeyboard(VARIANT_TRUE);
|
建议
如果你看到一个不知道的类型,别慌,去看一下文档。即使那个类型的名字里有 BOOL,但也不意味这你可以把 1 赋值给这个类型的变量。
在使用 HRESULT 这个类型的时候,有些程序员也会犯同样的错误。他们想要拿它来和 FALSE 和 TRUE 比较,但是忘记了:
|
#define S_OK ((HRESULT)0L)
#define S_FALSE ((HRESULT)1L)
|
所以我要提醒你要特别注意那些你没见过的类型,在编程的时候也别太着急。
27. 狡诈的BSTR字符串
让我们来讨论另一个令人讨厌的数据类型——BSTR(基础字符串「Basic string」或者二进制字符串「binary string」)。
下面的代码选自 VirtualBox 项目。这段代码中的错误被 PVS-Studio 分析器诊断为:V745 一个'wchar_t *'类型的字符串被错误的转换为‘BSTR’字符串。可以考虑使用函数 'SysAllocString' 。
|
....
HRESULT EventClassID(BSTR bstrEventClassID);
....
hr = pIEventSubscription->put_EventClassID(
L"{d5978630-5b9f-11d1-8dd2-00aa004abd5e}");
|
解释
BSTR是这样声明的:
|
typedef wchar_t OLECHAR;
typedef OLECHAR * BSTR;
|
让我们来讨论这个 BSTR 类型以对这个例子有一个更深的理解。
这个是
MSDN site
中 BSTR 的有关信息。读 MSDN 的文档确实不好玩,但不好玩也得做啊,是不?
BSTR (Basic string 或 binary string) 是COM,Automation 和 Interop 函数所使用的一种字符串数据类型。在所有可以进入脚本的接口中都可以使用 BSTR 数据类型。BSTR 的描述:
-
长度前缀。BSTR的前4个字节表示字符串长度。它的值不包含终止符null。
-
数据字符串。一个Unicode的字符串。可能会多次嵌入null字符。
-
终止符:两个null字符。
一个 BSTR 是一个指针。这个指针指向数据字符串的第一个字符,而不是长度前缀。BSTR 用 COM 内存分配函数类分配内存,所以它们可以从函数中返回而无需考虑内存分配。
下面的代码是不对的:
|
BSTR MyBstr = L"I am a happy BSTR";
|
这行代码可以正确构建(编译和链接),但是不会正确运行,因为这个字符串没有长度前缀。如果你用调试程序去检查这个变量的内存位置,你也不会在数据字符串前面找到一个4byte的长度前缀。所以,应该这样写:
|
BSTR MyBstr = SysAllocString(L"I am a happy BSTR");
|
现在定位这个变量的调试程序就能够找到一个包含值为34的长度前缀。是34而非17是因为前面有个‘L’修饰符,L把单字符(single-character)转换成了宽字符(single-character)。调试程序也能显示在数据字符串后面的 2byte 的终止符null。
COM 函数的参数应该是 BSTR,如果你用简单的 Unicode 字符串,COM 函数会出错的。
我想这些已经足够你了解,为什么我们要区分 BSTR 和简单的 "wchar_t *"字符串类型。
额外的链接:
-
MSDN.
BSTR
-
StackOverfow.
Static code analysis for detecting passing a wchar_t* to BSTR
.
-
StackOverfow.
BSTR to std::string (std::wstring) and vice versa
.
-
Robert Pittenger.
Guide to BSTR and CString Conversions
.
-
Eric Lippert.
Eric's Complete Guide To BSTR Semantics
.
正确代码
|
hr = pIEventSubscription->put_EventClassID(
SysAllocString(L"{d5978630-5b9f-11d1-8dd2-00aa004abd5e}"));
|
建议
建议跟前面的一样。当你看到不认识的数据类型的时候,别慌,去看看文档。这很重要,所以这个建议再次出现也无须大惊小怪的。
28. 能用简单函数完成的就别用宏
下面的代码选自 ReactOS 项目。代码中包含的错误被 PVS-Studio 诊断为:V640 代码的运算逻辑跟它的格式不一致。一直在执行第二句。可能是少了花括号。
|
#define stat64_to_stat(buf64, buf) \
buf->st_dev = (buf64)->st_dev; \
buf->st_ino = (buf64)->st_ino; \
buf->st_mode = (buf64)->st_mode; \
buf->st_nlink = (buf64)->st_nlink; \
buf->st_uid = (buf64)->st_uid; \
buf->st_gid = (buf64)->st_gid; \
buf->st_rdev = (buf64)->st_rdev; \
buf->st_size = (_off_t)(buf64)->st_size; \
buf->st_atime = (time_t)(buf64)->st_atime; \
buf->st_mtime = (time_t)(buf64)->st_mtime; \
buf->st_ctime = (time_t)(buf64)->st_ctime; \
int CDECL _tstat(const _TCHAR* path, struct _stat * buf)
{
int ret;
struct __stat64 buf64;
ret = _tstat64(path, &buf64);
if (!ret)
stat64_to_stat(&buf64, buf);
return ret;
}
|
解释
代码有点长。幸运的是,它也比较简单,所以应该不难理解。
大意是这样的,如果你成功地通过 _tstat64() 函数得到了文件的信息,那就把数据放到 of _stat 结构体中。我们用宏 stat64_to_stat 来保存数据。
这个宏不能正确运行。它执行的操作没有用花括号{}包起来。结果就是,条件运算符体只是宏里的第一个字符串。如果你展开那个宏,你会看到:
|
if (!ret)
buf->st_dev = (&buf64)->st_dev;
buf->st_ino = (&buf64)->st_ino;
buf->st_mode = (&buf64)->st_mode;
|
因此,不管有没有正确收到信息,结构体里的成员都会被复制。
这肯定是个错误,但在实际中,它不是致命的。我们比较幸运,这里只是白白复制未初始化的内存单元而已。但我有遇到过更严重的错误,也跟宏编写不当有关。
正确代码
最简单的改变就是给宏加个花括号。加do { .... } while (0)会更好。在宏和函数后面,你可以放个分号';'。
|
#define stat64_to_stat(buf64, buf) \
do { \
buf->st_dev = (buf64)->st_dev; \
buf->st_ino = (buf64)->st_ino; \
buf->st_mode = (buf64)->st_mode; \
buf->st_nlink = (buf64)->st_nlink; \
buf->st_uid = (buf64)->st_uid; \
buf->st_gid = (buf64)->st_gid; \
buf->st_rdev = (buf64)->st_rdev; \
buf->st_size = (_off_t)(buf64)->st_size; \
buf->st_atime = (time_t)(buf64)->st_atime; \
buf->st_mtime = (time_t)(buf64)->st_mtime; \
buf->st_ctime = (time_t)(buf64)->st_ctime; \
} while (0)
|
建议
我不能说我喜欢宏。我知道代码中不能没有宏,尤其是C。然而,如果可能我还是尽量避免使用它们。同时,我也呼吁你不要滥用宏。我不喜欢宏有三个原因:
-
代码会比较难调试。
-
容易出错。
-
代码变得更难理解,尤其是在一些宏中又调用其他的宏。
还有很多关于宏的错误。我给的这个例子已经能显示,我们不一定非要用宏不可。我实在无法理解为什么有的程序员不用简单的函数而要用宏。放弃用宏的优点:
-
代码更简单。你不需要花额外的时间来写宏,而且还要用‘\’来对齐。
-
代码更可读(例子里的错误不可能再在代码中出现)。
考虑到这些缺点,我只能想到优化。是,会有函数调用,但它不会那么危险。
无论如何,让我们假设这件事对我们来说很重要,然后考虑如何优化。首先,你可以用 inline。其次,把函数声明为static也不错。我觉得这对编译器编译这个函数,并且不用分离函数体已经够了。
事实上你一点也不用担心,因为现在的编译器都很智能。即使你的函数没有inline/static,但是如果它觉得应该加,编译器也会自己加进来的。不要被这种细节困扰。最好还是写一个简单易懂的函数,这样更有利。
在我看来,上面的代码应该这样写:
|
static void stat64_to_stat(const struct __stat64 *buf64,
struct _stat *buf)
{
buf->st_dev = buf64->st_dev;
buf->st_ino = buf64->st_ino;
buf->st_mode = buf64->st_mode;
buf->st_nlink = buf64->st_nlink;
buf->st_uid = buf64->st_uid;
buf->st_gid = buf64->st_gid;
buf->st_rdev = buf64->st_rdev;
buf->st_size = (_off_t)buf64->st_size;
buf->st_atime = (time_t)buf64->st_atime;
buf->st_mtime = (time_t)buf64->st_mtime;
buf->st_ctime = (time_t)buf64->st_ctime;
}
|
说实话,在这里我们还可以做更多的改进。例如在C++中,最好不用指针而用引用。用一个没有预先检查的指针看上去不会有多好看。但这又是另一个故事了,我不会在讲宏的部分里讨论它的。
29. 在迭代器中使用前自增(++i)而不是用后自增(i++)
下面的代码选自 Unreal Engine 4 项目。这段代码被 PVS-Studio 诊断为不够高效,因为:V803 性能下降。在 ‘itr’ 是迭代器的例子中,使用前自增比用后自增高效。把 iterator++ 换成 ++iterator。
|
void FSlateNotificationManager::GetWindows(....) const
{
for( auto Iter(NotificationLists.CreateConstIterator());
Iter; Iter++ )
{
TSharedPtr
NotificationList = *Iter;
....
}
}
|
解释
如果你没有看到这一节的标题,我想你应该很难注意到这段代码的问题。乍一看,这段代码没什么问题,但是还不够完美。是的,我在讨论后自增——“Iter++”。相对于后自增迭代器,你更应该用前自增的形式,比如用‘++Iter’代替‘ Iter++’。为什么我们要这么做呢,这么做的现实意义在哪?这里面是有玄机的。
高效代码
|
for( auto Iter(NotificationLists.CreateConstIterator());
Iter; ++Iter)
|
建议
前自增和后自增的区别大伙儿都知道。我希望大家也同样知道这两个结构内部的区别(这个区别表现了我们运算的规则)。如果你曾经重载过这个运算,你肯定有注意到。如果没有——我会做一个简单的解释。(了解的人可以直接跳过这一段,直接跳到代码后面的。)
前自增运算会改变对象的状态,然后给自己返回改变过的值。不需要临时对象。前自增运算大概是这样的:
|
MyOwnClass& operator++()
{
++meOwnField;
return (*this);
}
|
后自增同样也改变对象的状态,但是它返回的是对象之前的状态。它能做到返回之前状态是用到了临时对象。后自增重载代码应该是这样的:
|
MyOwnClass operator++(int)
{
MyOWnCLass tmp = *this;
++(*this);
return tmp;
}
|
通过上面的代码,你可以看到有一个加法运算用到了临时变量。在实际应用中,这有多重要呢?
现在的编译器都很智能,能自己做优化,而且如果临时变量没有用的话,它们就不会去创建它。这就是为什么发布版本很难找出'it++' 和 '++it'的区别。
但是在调试的时候,又不一样了。在这个例子中,这两种表现的区别还是很重要的。
比如说,在这篇文章中,就用了几个例子测试代码在调试版本中用前自增和后自增所用的时间。我们可以看到用后自增时间差不多要长四倍。
有些人会说,“然后呢?他们在发布版本中是一样的!”,在他们这么说也对也不对。一般来说,在做单元测试的时候,我们会花比较多的时间在调试版本上去调试代码。虽然我们要花很长的时间在软件的调试版本上,但并不意味着我们愿意浪费时间呀。
总的来说,我觉得我们已经解决这个问题了。——我们应该用前自增代替后自增。是的,你应该。这样在调试的时候你花的时间就比较少了啊。而且,如果迭代器非常‘大量’,效果就更可观了。
参考文献(建议阅读):
30. Visual C++和wprintf()函数
下面的代码选自 Energy Checker SDK. 代码中包含的错误被 PVS-Studio 诊断为:V576 不正确的格式。建议检查 'wprintf' 函数的第二个参数。需要指向wchar_t 字符串的指针标志。
|
int main(void) {
...
char *p = NULL;
...
wprintf(
_T("Using power link directory: %s\n"),
p
);
...
}
|
解释
注意:第一个错误是用
_T
来标识一个一个宽字符的字符串格式。应该用前缀L才对。然而,这个错误不是太严重,也不是我们的兴趣所在。如果我们不用宽字符格式,代码只是不能编译而已,_T也不会扩展的。