关于编程、重构等 42条建议上（~持更）

2019-11-08 01:40:33

字体：大中小

来源：转载

供稿：网友

原文来自： The Ultimate Question of PRogramming, Refactoring, and Everything https://software.intel.com/en-us/articles/the-ultimate-question-of-programming-refactoring-and-everything

译文来自：http://blog.csdn.net/huanglong8/article/details/56029783

主要说了在编程方面的42条建议，这些建议可以帮助你避免错误，节省时间和精力。原作者是 Andrey Karpov - “Program Verification Systems”的技术总监，它的团队致力于研发PVS-Studio静态代码分析器。检查了大量的开源项目后，整理出了一些建议，并且每个建议都给出了一个实际的例子，以此说明这个问题的现实性。以 C / C ++程序员为主，但通常可以应用所有范例中。这里写图片描述

前言

作者是C/C++语言的牛人，是微软VC++ MVP，作者写作的意图就是让你写出更安全，更高效的C代码，作者也喜欢分享，才有了这42条建议。

1. 不要去做编译器优化的工作

此内容来自MySQL项目，在PVS-Studio分析器中诊断出如下错误：

static int rr_cmp（uchar * a，uchar * b）{ if（a [0]！= b [0]） return（int）a [0] - （int）b [0]; if（a [1]！= b [1]） return（int）a [1] - （int）b [1]; if（a [2]！= b [2]） return（int）a [2] - （int）b [2]; if（a [3]！= b [3]） return（int）a [3] - （int）b [3]; if（a [4]！= b [4]） return（int）a [4] - （int）b [4]; if（a [5]！= b [5]） return（int）a [1] - （int）b [5]; <<<< ==== if（a [6]！= b [6]） return（int）a [6] - （int）b [6]; return（int）a [7] - （int）b [7];}}

说明观察上述代码，其实外人也不敢保证它是对的还是错的，但经过mysql上下文分析后，发现，偶尔返回错误的值，原因就是，程序员复制了代码块“if（a [1]！= b [1]）return（int）a [1] - （int）b [1];”。然后他开始更改索引，忘记将“1”替换为“5”。这种小的且偶发性的逻辑错误很难找到。

正确代码

if（a [5]！= b [5]） return（int）a [5] - （int）b [5];

建议我们当初在思考为什么不写成循环时，我们得知，程序员为了优化代码，帮助编译器做了展开循环的工作。当然在实际开发中，我们可能也会这样做，来提升轻微的速度或是减少内存。但现在的编译器都很聪明，为什么非要这么做。如果一个简单的循环可以帮助理解，并减少犯错的机会，那不是很好？

static int rr_cmp（uchar * a，uchar * b）{ for（size_t i = 0; i <7; ++ i） { if（a [i]！= b [i]） return a [i] - b [i]; }} return a [7] - b [7];}}

它的优点就是易于阅读和理解，并且不太可能会写错。通常来讲简单的代码通常才是正确的代码，不要尝试去做编译器做的工作。

2. >0与==1是两码事

代码片段来自 CoreCLR项目，其中针对memcmp(…) == -1做了说明，当然还有诸如此类的函数。

bool Operator（）（const GUID＆_Key1，const GUID＆_Key2）const {return memcmp（＆_ Key1，＆_Key2，sizeof（GUID））== -1; }}

说明 int memcmp（const void * ptr1，const void * ptr2，size_t num）; 是将ptr1和ptr2中的前num字节进行比较，内存内容相等返回0，不等则返回非0。 <0 - 前num的第一个不同字节的数值比较，ptr1的小（如果计算为unsigned char值）。 == 0 - 两个内存块的内容相等。 >0 - 前num的第一个不同字节的数值比较，ptr1的大（如果计算为unsigned char值）。不要将诸如memcmp（），strcmp（），strncmp（）等函数的结果与常量1和-1进行比较。有趣的是，很多代码都这么写，并且还长期的运行着，从来没有出错，那我认为那是运气，因为当这些函数的行为被更改。例如换编译器，或重写事先，将导致你的代码出bug。 正确代码

bool operator（）（const GUID＆_Key1，const GUID＆_Key2）const {return memcmp（＆_Key1，＆_Key2，sizeof（GUID））<0; }}

建议遵从函数确定的实现及返回，不要依赖于经验等，如果文档说返回大于小于0，也不要将他和特定的数字进行比较。顺便讲一下，这个函数返回1024的实际情况，这个错误是mysql/MariaDB中导致的，主要是由于访问后返回的token对其256位内进行比较，一旦超出，则永远都会返回true，哪怕这个人不知道密码。在passWord.c中

my_bool check（...）{ return memcmp（...）;

有关这个问题更详述的信息参考：MySQL / MariaDB中的安全漏洞。

3. 复制代码，检查多次

这个错误来自Audacity项目，代码如下：

sampleCount VoiceKey :: OnBackward（....）{ ... ... int atrend = sgn（buffer [samplesleft-2] - buffer [samplesleft - 1]）; int ztrend = sgn（buffer [samplesleft-WindowSizeInt-2] - buffer [samplesleft - WindowSizeInt-2]）; ... ...}}

说明其实一眼看去，如果你不是这个项目的参与者，或者是刚接手这个项目，你很难明白错误在哪里，即便指出你也不能保证这就是错误。buffer [samplesleft-WindowSizeInt-2]出现此错误同样因为复制粘贴，程序员复制了代码字符串，但忘记将2替换为1。这是一个很low的错误，但我们每个人都会犯，这也就是为什么强调的原因。 正确的代码

int ztrend = sgn（buffer [samplesleft-WindowSizeInt-2] - buffer [samplesleft - WindowSizeInt-1]）;

建议复制代码快可以帮助你高效的完成敲码工作，但请你要多检查他几次，因为那些实体，数字一旦反生bug，则将很难被找出，稍后我们还是会讨论复制粘贴，让你印象深刻。

4. ？:三目运算符括起来用

片段来自Haiku项目，？：运算符优先级低于-运算符导致。

bool IsVisible（bool ancestorsVisible）const{ int16 showLevel = BView :: Private（view）.ShowLevel（）; return（showLevel - （ancestorsVisible）？0：1）<= 0;}}

说明一同来看看C/C++运算符优先级。三目运算符具有非常低的优先级，低于/+<等等，它也比负优先级低，因此，当混合使用时，需要更加注意。程序员认为运算顺序是：

（showLevel - （ancestorsVisible？0：1））<= 0

然后呵呵

（（showLevel - ancestorsVisible）？0：1）<= 0

错误明显，这也是很简单的代码，却很容易犯错，如果你有包含三目的非常复杂的算式，那。。。这么难读就不要搞这么长的算式。 正确的代码

return showLevel - （ancestorsVisible？0：1）<= 0;

建议如果你的三目混合其中，那么请加上括号，以避免这种错误，也帮助程序员去阅读你的代码。当然我也不排斥直接使用三目，当然，只有你需要一个表达式，表达式中只有三目时，你可以不写括号，这样看起来更简洁。

5. DllMain中不要做太多复杂的事

此段来自LibreOffice项目，由PVS检测出，CreateThread函数不应该由DllMain函数调用。

BOOL WINAPI DllMain（HINSTANCE hinstDLL， DWORD fdwReason，LPVOID lpvReserved）{ .... CreateThread（NULL，0，ParentMonitorThreadProc，（LPVOID）dwParentProcessId，0，＆dwThreadId）; ....}}

说明在一定条件下，DllMain不得不做一些事情，当我在这个主函数下调用Win API时，莫名奇妙的发现它并不工作，我也没设法弄清楚当时的问题。再进行PVS开发工作时，我突然意识到之前失败的原因。在DllMain函数中，你只能执行非常有限的操作，原因就是Dll可能还没有完全被加载到内存中，你无法在这里面调用其他的API。当然，现在诊断出的结果是警告，以此来提醒程序员，是不是应该放到其他中去。细节使用DllMain更多的细节应该参考MSDN上的文章，动态链接库实践。这里列举一些摘要。

DllMain被调用时内存加载处于锁定状态，这意味着在它之中调用的函数有一定的限制，因此，在主函数入口中，应该只进行一小部分的初始化任务，如果你调用了任何直接或间接尝试获取加载器锁的函数，那么程序可能会导致死锁或崩溃，而这种错误可能直接影响到整个进程或线程中。某DllMain的一个好的初始化方法就是尽可能延长初始化，这样使您更安全的使用API。当然不是所有任务都可以延长初始化的，如果某些不正确的资源或配置，应该立即退出，而不是通过做其他的工作继续浪费时间。

不要在DllMain中执行以下任务：

不要直接或间接的调用LoadLibrary或LoadLibraryEx，导致死锁或崩溃滴。不要直接或间接的调用GetStringTypeA，GetStringTypeEx或GetStringTypeW，同上滴。与其他线程同步，同上滴。获取或等待同步对象，锁啊这类的释放，同上滴。使用ColnitializeEx初始化COM线程。调用注册表函数。这个是加载advapi32.dll后才能用的。调用CreateProcess。如果这个进程又加载另一个DLL。调用ExitThread。在卸载期间如果推出线程，哦哦。调用CreateThread。如果不同步，是可以的，就是有点风险。创建命名管道或其他对象（限win2000），命名对象由终端服务DLL提供。如果这个Dll没有初始化，则可能哦崩溃。使用CRT的内存管理。如果CRT的DLL未初始化，也有问题。调用User32.dll或Gdi32.dll中的函数，因为这里面的函数可能加载了另一个dll。使用托管代码。 正确的代码 LibreOffice的代码可能不工作，这是偶发的。修复这个不简单，因为要重构，以使DllMain函数尽可能短。建议很难建议，因为谁知道突然发生个莫名其妙的bug，但读完这里，你至少知道，在写这样的代码时，应该注意哪些问题，从而避免。还有不要告诉我在dllmain要干嘛干嘛，我也不知道，这一切都得靠你。

6. 指针被显式转换为整型

此片段来自ipP Samples项目，看这代码

void write_output_image（....，const Ipp32f * img， ....，const Ipp32s iStep）{ ... ... img =（Ipp32f *）（（unsigned long）（img）+ iStep）; ... ...}}

说明程序员将指针移动一定数量的字节，此代码在win32下正确，因为指针大小和long类型同样。但是如果换成64位版本，就会造成高位丢失。 linux使用不同的数据类型。可能也会有类似问题，但很多linux程序也都会进入到windows中，所以，还是使用intprt_t这样的类型来定义比较妥。此错误如图：这里写图片描述这种错误有时也很难注意到，因为导致指针高位丢失也只能是在程序运行了好长时间后才偶然出现的问题。 正确的代码 使用size_t，INT_PTR，DWORD_PTR，intrptr_t来定义。