标签 Cpp 下的文章

给新手程序员的建议

本文翻译自Dr. Dobb’s杂志主编Andrew Binstock的"Advice to a new programmer"一文

总是有太多的建议摆在新手程序员面前,以致他们难于选择从何处开始。然而,所有这些建议都是建构在下面这五条实践的基础之上的。

每隔几个月,我就会收到一些勤奋有加的新手程序员的求助,他们希望知道如何才能成为一名真正优秀的程序员。在一些程序员论坛上,我也能看到为数不 少的类似问题,这是一个令人鼓舞的趋势。一些最周全的答案往往与我对这个问题的看法相似,这表明在基础的最佳实践上确实存在着某种一致。因此我下 面的建议并非原创,不过也许我的这些补充会提供给你更进一步的理解。

我印象中的新手程序员基本了解编程的原理,写过程序,但大多规模较小且复杂度不一,致力于某个领域的工作或自己或他人个人项目。

编程工作只有一种真正的基础活动,那就是写代码。要想擅于编程,你就必须编写大量的代码。 大量的工作可能成为一种促进你成长的工具,也可能是一些有限技能的重复练习。为了避免成为后者,你应该做到:

阅读大量代码。尤其是阅读大量由卓越程序员编写的代码。记住:不仅读那些坐在大厅里的优秀程序员的代码,更要读那些卓越程序员的。 在开源软件大行其道的今天,做到这些十分容易。当我学习Java时,我读了Tomcat的代码,读了Cruise Control CI服务器的代码。自从那时起,我已经读了大量优秀的代码。

阅读代码时很容易从main函数开始,但这样一来,你很可能会在初始化以及命令行解析代码上花费大量时间。我更喜欢根据源文件名寻找一些令我感兴 趣的功能实现,然后深入阅读这些源文件。理解整个项目或整体设计的来龙去脉并不是关键,做这些会让你感觉筋疲力尽的。阅读代码。查看注释,弄清楚 作者在做什么以及是如何着手做的。

彻底了解你的工具。我认为损耗编程时间最多的不是调试或重写代码,而是因开发人员对其所使用工具的不熟悉而导致的无数碎片时间的损 耗。我所指的工具包括:集成开发环境(IDE)、编程语言、构建系统以及版本控制系统。其中,集成开发环境和编程语言是到目前为止最为重要的。经 过几个星期的练习,你应该知道集成开发环境中的几乎每个按键组合,这样你只有在为了节省按键时间时才使用鼠标。如果你知道按键组合,你就知道了这 些命令。但如果你只使用鼠标,你只会知道菜单,菜单上面有你倾向于点击的相同的一个或两个条目。因此了解集成开发环境是一种不折不扣的纪律。

了解大型编程语言,诸如Java或C++,需要的不仅仅是纪律。它们自身规模庞大,它们的库亦规模庞大。阅读代码是我认为的了解编程语言最好的方式,阅读 那些使用了你所未知特性的代码并寻找机会使用它们。书籍(而不是博客)是另外一个极好的资料来源。了解你目前正在使用的特性的外围,很快你就会发现外围扩 大了。了解版本控制系统和构建系统将让你成为一个理想的团队成员 — 不会因为对重要操作的无知而浪费时间。

动手编码前先规划好你的代码。我认为这是建议列表中最难做的一项,但同时它也可能给你换来最多的益处。我所指的并不是正式的设计 – 在这个阶段正式设计一般不是必要的。不过你确实应该用一种其他方式精心策划一下代码,而不仅仅是将思路放在脑子里。最简单的方法就是编写一个小文档(我经 常使用的是思维导图):代码的需求是什么?你打算如何实现它?还有哪些目前未知的事情需要去了解?我需要或需要创建什么对象?将这些都写出来。只有在这样 之后开始编码,你才会发现代码变得更加容易编写了,更容易形成文档了,也更容易修改了。将你的笔记保存下来 – 它们将是很好的参考资料。

大量编写代码并进行代码评审。如果你那里不做代码评审,那你就自己来做。找出那些最好的程序员,并且你可以通过某种方式听 到和理解他们给你的有用的建议。别做令人讨厌的人,但也不能因为你害羞,忙碌或自负而回避这个过程。代码评审应该成为你编程人生的一部分。要有创造性。试 试在某个下午与比你更牛的程序员一起做结对编程。重要的是你需要反馈,而这些反馈是你自己无法给予自己的。

编码与写测试并驾齐驱。这也许是这里唯一有争议的一条。它并非是对TDD的认可(译注:测试驱动开发)。但这里要认可的是你的代码 在大多数要面对的场合里都是可以工作的。开始单元测试,并用边缘值测试新代码。例如,当传入一个负数或者是整型数最大值时,你的函数是否还能正常工作?如 果不能,你的函数是否抛出了一个信息详实的异常或只是崩溃退出?如果不是异常,你是否使用了断言(assert)来缩小输入的范围?如果这么做了,那就测 试这个断言。利用之前做的规划编写一些模拟(mock)测试,接下来用这些模拟对象去开始测试你的新代码。这将有助于阐明你当前代码中的设计问题以及即将 实现的对象。保存你的测试代码,在每次签入代码前都运行它们,这些测试将成为后续那些破坏你当前代码的新代码的早期预警系统。

还有许多建议和至理名言可以添加到这个列表中。但这本身就是问题的一部分:过多的建议将导致新手难于知道到底从何处开始。因此,我故意将我的建议缩减到仅 剩五点。如果你能勤奋地运用这五点建议实践,你会很快发现两件事情:你将可以逐步应付更大更重要的任务了,并且当你回首翻看你几个月之前编写的代码时,你 会觉得尴尬。

毫无疑问这两种感受都是你进步的标志。祝你好运!

C,C++开源项目中的100个Bugs

俄罗斯OOO Program Verification Systems公司用自己的静态源码分析产品PVS-Studio对一些知名的C/C++开源项目,诸如Apache Http ServerChromiumClangCMakeMySQL等的源码进行了分析,找出了100个典型的Bugs。个人觉得这份列表对C/C++ 程序员有一定参考意义。与其说事后用静态工具分析,倒不如在编码时就提高自知自觉,避免这份列表上的错误发生在你的代码中,因此这里将部分摘录一些Bugs(Bug编号这里不连续,为的是对应原文的编号)并做简要说明。原文将这份Bug列表分为了几类,这里也将沿用这个思路。

一、数组和字符串处理错误

数组和字符串处理错误是C/C++程序中最多的一类缺陷类型。这也可以看作是我们为拥有高效地底层内存操作能力而付出的代价。

[#1] Wolfenstein 3D项目 -"只有部分对象被clear了"

void CG_RegisterItemVisuals( int itemNum ) {
    …
    itemInfo_t *itemInfo;
    …
    memset( itemInfo, 0, sizeof( &itemInfo ) );
    …
}

这里的Bug出现在memset那一行。代码的真实意图是clear iteminfo这块内存,但调用memset时,第三个参数传入的却是sizeof(&iteminfo),要知道 sizeof(&itemInfo) != sizeof(itemInfo_t),前者只是一个指针的大小罢了。正确的写法是:

memset(itemInfo, 0, sizeof(itemInfo_t)); 或memset(itemInfo, 0, sizeof(*itemInfo));

[#2] Wolfenstein 3D项目 -"只有部分Matrix被clear了"

ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
    memcpy( mat, src, sizeof( src ) );
}

这里的Bug出现在memcpy一行。程序的原意是将clear src[3][3]这个二维数组。但这里有个坑:那就是作为函数形式参数的数组名已经退化为指针了,对其sizeof只能得到一个指针的长度,因此这里的 memcpy只是copy了一个指针的长度,没有copy全。这里的代码是C++代码,原文中给出了正确的改正方法 – 传reference:

ID_INLINE mat3_t::mat3_t( float (&src)[3][3] )
{
    memcpy( mat, src, sizeof( src ) );
}

[#4] ReactOS项目 – "错误地计算一个字符串的长度"

static const PCHAR Nv11Board = "NV11 (GeForce2) Board";
static const PCHAR Nv11Chip = "Chip Rev B2";
static const PCHAR Nv11Vendor = "NVidia Corporation";

BOOLEAN
IsVesaBiosOk(…)
{
    …
    if (!(strncmp(Vendor, Nv11Vendor, sizeof(Nv11Vendor))) &&
            !(strncmp(Product, Nv11Board, sizeof(Nv11Board))) &&
            !(strncmp(Revision, Nv11Chip, sizeof(Nv11Chip))) &&
            (OemRevision == 0×311))
    …
}

Bug处在IsVesaBiosOK中那一串strncmp调用中,代码将一个指针的size传入strncmp作为第三个参数,导致 strncmp实际只是比较了字符串的前4 or 8个字节,而不是字符串的全部内容。

[#6] CPU Identifying Tool项目 – 数组越界

#define FINDBUFFLEN 64  // Max buffer find/replace size

int WINAPI Sticky (…)
{
    …
    static char findWhat[FINDBUFFLEN] = {'\0'};
    …
    findWhat[FINDBUFFLEN] = '\0';
    …
}

bug出在"findWhat[FINDBUFFLEN] = ‘\0′;”这一行。数组的最大长度为FINDBUFFLEN,但下标的最大值应该是FINDBUFFLEN-1,而不是FINDBUFFLEN。因此这 行代码显然应该改为findWhat[FINDBUFFLEN-1] = '\0';

[#7] Wolfenstein 3D项目 – 数组越界

typedef struct bot_state_s
{
    …
    char teamleader[32]; //netname of the team leader
    …
}  bot_state_t;

void BotTeamAI( bot_state_t *bs ) {
    …
    bs->teamleader[sizeof( bs->teamleader )] = '\0';
    …
}

"sizeof( bs->teamleader )]"这行的结果值已经超出了数组的最大边界,正确的代码是:

bs->teamleader[
  sizeof(bs->teamleader) / sizeof(bs->teamleader[0]) – 1
  ] = '\0';

[#8] Miranda IM项目 – 只Copy了部分字符串

struct _textrangew
{
    CHARRANGE chrg;
    LPWSTR lpstrText;
} TEXTRANGEW;

const wchar_t* Utils::extractURLFromRichEdit(…)
{
    …
    ::CopyMemory(tr.lpstrText, L"mailto:", 7);
    …
}

这里的bug在于L"mailto:"是宽字符串,宽字符串中的每个字符占2或4个字节(依Compiler使用的字符集编码而定),因此这里只 copy 7个字节显然是不够的,应该是7 * sizeof(wchar_t)。

[#9] CMake项目 – 循环內的数组越界

static const struct {
    DWORD   winerr;
    int     doserr;
} doserrors[] =
{
    …
};

static void
la_dosmaperr(unsigned long e)
{
    …
    for (i = 0; i < sizeof(doserrors); i++)
    {
        if (doserrors[i].winerr == e)
        {
            errno = doserrors[i].doserr;
            return;
        }
    }
    …
}

作者原本意图la_dosmaperr中for循环的次数等于数组的元素个数,但sizeof(doserrors)返回的却是数组占用的字节个数,这远远大于数组元素个数,因此造成数组越界。正确的写法:

for (i = 0; i < sizeof(doserrors) / sizeof(*doserrors); i++)

[#10] CPU Identifying Tool项目 – 打印到自身的字符串

char * OSDetection ()
{
    …
    sprintf(szOperatingSystem,
                    "%sversion %d.%d %s (Build %d)",
                    szOperatingSystem,
                    osvi.dwMajorVersion,
                    osvi.dwMinorVersion,
                    osvi.szCSDVersion,
                    osvi.dwBuildNumber & 0xFFFF);
    …
    sprintf (szOperatingSystem, "%s%s(Build %d)",
                      szOperatingSystem, osvi.szCSDVersion,
                      osvi.dwBuildNumber & 0xFFFF);
    …
}

通过sprintf,szOperatingSystem字符串将自己打印到自己里面,这是十分危险的,将导致无法预知的错误结果,可能会导致栈溢出等严重问题。

[#12] Notepad++项目 – 数组局部clear

#define CONT_MAP_MAX 50
int _iContMap[CONT_MAP_MAX];

DockingManager::DockingManager()
{
    …
    memset(_iContMap, -1, CONT_MAP_MAX);
    …
}

代码的原本试图将数组_iContMap清零,但memset的第三个参数CONT_MAP_MAX并不能代表数组的真正大小,而只是数组的元素个数而已,显然其忘记乘以sizeof(int)了。

二、未定义行为

在C/C++的语言规范中,我们常常能看到“xx is undefined”。规范中并没有明确表明这类错误是什么样子的,只是说取决于Compiler的实现,也许Compiler会给出正确的结果,但这么使用却是不可移植的。

[#1] Chromium项目 – 智能指针的误用

void AccessibleContainsAccessible(…)
{
    …
    auto_ptr<VARIANT> child_array(new VARIANT[child_count]);
    …
}

这里的问题在于使用new[]分配的内存,在智能指针释放时却用了delete,这将会导致未定义行为。看看autoptr的destructor就知道了:

~auto_ptr() {
    delete _Myptr;
}

我们可以找一些更合适的类来fix这个问题,比如boost::scopedarray。

[#2] IPP Sample项目 – 经典未定义行为

template<typename T, Ipp32s size> void HadamardFwdFast(…)
{
  Ipp32s *pTemp;
  …
  for(j=0;j<4;j++) {
    a[0] = pTemp[0*4] + pTemp[1*4];
    a[1] = pTemp[0*4] – pTemp[1*4];
    a[2] = pTemp[2*4] + pTemp[3*4];
    a[3] = pTemp[2*4] – pTemp[3*4];
    pTemp = pTemp++;
    …
  }
  …
}

很多人一眼就看到了"pTemp = pTemp++"这行,对于这个代码编译器会产生两种结果截然不同的翻译:

pTemp = pTemp + 1;
pTemp = pTemp;

TMP = pTemp;
pTemp = pTemp + 1;
pTemp = TMP;

到底是哪种呢?依赖于编译器的实现,甚至是优化级别的设定。

三、与运算优先级相关的错误

[#1] MySQL工程 – !和&的运算优先级

int ha_innobase::create(…)
{
  …
  if (srv_file_per_table
            && !mysqld_embedded
            && (!create_info->options & HA_LEX_CREATE_TMP_TABLE)) {
  …
}

这段代码原意是想测试create_info->options变量中几个bit位的值是否set了,即!(create_info->options & HA_LEX_CREATE_TMP_TABLE),但由于!的运算优先级高于&,实际逻辑变成了(!create_info->options) & HA_LEX_CREATE_TMP_TABLE了。如果想要这段代码如期工作,就不要吝啬小括号了。

[#2] Emule工程 – *和++的运算优先级

STDMETHODIMP
CCustomAutoComplete::Next(…, ULONG *pceltFetched)
{
  …
  if (pceltFetched != NULL)
    *pceltFetched++;
  …
}

显然作者原意是想对pceltFetched所指向的long型变量进行++操作,但由于*和++的运算优先级没有搞对,导致实际上执行了*(pceltFetched++)的操作,而不是(*pceltFetched)++操作。

[#3] Chromium项目 – &和!=的运算优先级

#define FILE_ATTRIBUTE_DIRECTORY 0×00000010

bool GetPlatformFileInfo(PlatformFile file, PlatformFileInfo* info) {
  …
  info->is_directory =
    file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0;
  …
}

这个程序员的意图是通过测试file_info.dwFileAttributes的几个bit位的值来判定是否是目录,逻辑上应该是(file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0,但由于!=优先级高于&,原代码中无括号,结果逻辑变成了file_info.dwFileAttributes & (FILE_ATTRIBUTE_DIRECTORY != 0),导致is_directory将永远求值为true。

[#4] BCmenu项目 – if和else弄混

void BCMenu::InsertSpaces(void)
{
  if(IsLunaMenuStyle())
    if(!xp_space_accelerators) return;
  else
    if(!original_space_accelerators) return;
  …
}

这又是C语言的一个“大坑”,无奈这个BCMenu项目的程序员掉坑里了。虽然从代码缩进上来看,else似乎是与最外层的if配对使用,但实际这段代码的效果是:

if(IsLunaMenuStyle())
{
   if(!xp_space_accelerators) {
     return;
   } else {
     if(!original_space_accelerators) return;
   }
}

这显然不是程序员原意,看来括号必要时还是不能省略的。修改后的代码如下:

if(IsLunaMenuStyle()) {
  if(!xp_space_accelerators) return;
} else {
  if(!original_space_accelerators) return;
}

四、格式化输出错误

[#1] ReactOS项目 – 错误地输出WCHAR字符

static void REGPROC_unescape_string(WCHAR* str)
{
  …
  default:
    fprintf(stderr,
            "Warning! Unrecognized escape sequence: \\%c'\n",
            str[str_idx]);
  …
}

%c是用来格式化输出非宽字符的,这里用来输出WCHAR显然会得到错误的结果,fix solution是将%c换位%C。

[#2] Intel AMT SDK项目 – 缺少%s

void addAttribute(…)
{
  …
  int index = _snprintf(temp, 1023, 
                        "%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
                        "%02x%02x:02x%02x:%02x%02x:%02x%02x",
                        value[0],value[1],value[2],value[3],value[4],
                        value[5],value[6],value[7],value[8],
                        value[9],value[10],value[11],value[12],
                        value[13],value[14],value[15]);
  …
}

 

不解释了,自己慢慢数和对照吧。

[#3] Intel AMT SDK项目 – 未使用的参数

bool GetUserValues(…)
{
  …
  printf("Error: illegal value. Aborting.\n", tmp);
  return false;
}

显然tmp是多余的。

五、书写错误

[#1] Miranda IM项目 – 在if中赋值

void CIcqProto::handleUserOffline(BYTE *buf, WORD wLen)
{
  …
  else if (wTLVType = 0×29 && wTLVLen == sizeof(DWORD))
  …
}

“wTLVType = 0×29”显然是笔误,应该是“wTLVType == 0×29”才对。

[#3] Clang项目 – 对象名书写错误

static Value *SimplifyICmpInst(…) {
  …
  case Instruction::Shl: {
    bool NUW =
      LBO->hasNoUnsignedWrap() && LBO->hasNoUnsignedWrap();
    bool NSW =
      LBO->hasNoSignedWrap() && RBO->hasNoSignedWrap();
  …
}

从最后一行先后使用了LBO和RBO来看,前面只用了LBO的那行很可能是有问题的,正确的应该是:

bool NUW =
      LBO->hasNoUnsignedWrap() && RBO->hasNoUnsignedWrap();

[#6] G3D Content Pak项目 – 一对括号放错了地方

bool Matrix4::operator==(const Matrix4& other) const {
  if (memcmp(this, &other, sizeof(Matrix4) == 0)) {
    return true;
  }
  …
}

由于括号放错了地方,导致memcmp最后的参数变成了sizeof(Matrix4) == 0,这行代码的正确写法应该是:

if (memcmp(this, &other, sizeof(Matrix4)) == 0) {

[#8] Apache Http Server项目 – 多余的sizeof

PSECURITY_ATTRIBUTES GetNullACL(void)
{
  PSECURITY_ATTRIBUTES sa;
  sa  = (PSECURITY_ATTRIBUTES)
    LocalAlloc(LPTR, sizeof(SECURITY_ATTRIBUTES));
  sa->nLength = sizeof(sizeof(SECURITY_ATTRIBUTES));
  …
}

最后一行显然是笔误,sizeof(sizeof(SECURITY_ATTRIBUTES))应该写为sizeof(SECURITY_ATTRIBUTES)才对。

[#10] Notepad++项目 – 在本来应该用&的地方使用了&&

TCHAR GetASCII(WPARAM wParam, LPARAM lParam)
{
  …
  result=ToAscii(wParam,
                 (lParam >> 16) && 0xff, keys,&dwReturnedValue,0);
  …
}

(lParam >> 16) && 0xff没有什么意义,求值结果总是true。这里的代码应该是(lParam >> 16) & 0xff。

[#12] Fennec Media Project项目 – 额外的分号

int settings_default(void)
{
  …
  for(i=0; i<16; i++);
    for(j=0; j<32; j++)
    {
      settings.conversion.equalizer_bands.boost[i][j] = 0.0;
      settings.conversion.equalizer_bands.preamp[i]   = 0.0;
    }
}

这又是一个实际逻辑与代码缩进不符的例子。作者的原意是这样的:

for(i=0; i<16; i++) 
{
    for(j=0; j<32; j++)
    {
      settings.conversion.equalizer_bands.boost[i][j] = 0.0;
      settings.conversion.equalizer_bands.preamp[i]   = 0.0;
    }
}

但实际执行代码逻辑却是:

for(i=0; i<16; i++) 
{
    ;
}

for(j=0; j<32; j++)
{   
  settings.conversion.equalizer_bands.boost[i][j] = 0.0;
  settings.conversion.equalizer_bands.preamp[i]   = 0.0;
}

这一切都是那个;导致的。

六、对基本函数和类的误用

[#2] TortoiseSVN项目 – remove函数的误用

STDMETHODIMP CShellExt::Initialize(….)
{
  …
  ignoredprops = UTF8ToWide(st.c_str());
  // remove all escape chars ('\\')
  std::remove(ignoredprops.begin(), ignoredprops.end(), '\\');
  break;
  …
}

作者意图删除所有'\\',但他用错了函数,remove函数只是交换元素的位置,将要删除的元素交换到尾部trash,并且返回指向trash首地址的iterator。正确的做法应该是"v.erase(remove(v.begin(), v.end(), 2), v.end())"。

[#5] Pixie项目 – 在循环中使用alloca函数

inline  void  triangulatePolygon(…) {
  …
  for (i=1;i<nloops;i++) {
    …
    do {
      …
      do {
        …
        CTriVertex  *snVertex =
         (CTriVertex *)alloca(2*sizeof(CTriVertex));
        …
      } while(dVertex != loops[0]);
      …
    } while(sVertex != loops[i]);
    …
  }
  …
}

alloca函数在栈上分配内存,因此在循环中使用alloca可能会很快导致栈溢出。

七、无意义的代码

[#1] IPP Samples项目 – 不完整的条件

void lNormalizeVector_32f_P3IM(Ipp32f *vec[3],
                                 Ipp32s* mask, Ipp32s len)
{
  Ipp32s  i;
  Ipp32f  norm;

  for(i=0; i<len; i++) {
    if(mask<0) continue;
    norm = 1.0f/sqrt(vec[0][i]*vec[0][i]+
                     vec[1][i]*vec[1][i]+vec[2][i]*vec[2][i]);
    vec[0][i] *= norm; vec[1][i] *= norm; vec[2][i] *= norm;
  }
}

mask是Ipp32s类型指针,这样if (mask< 0)这句代码显然没啥意义,正确的代码应该是:

if (mask[i] < 0) continue;

[#2] QT项目 – 重复的检查

Q3TextCustomItem* Q3TextDocument::parseTable(…)
{
  …
  while (end < length
         && !hasPrefix(doc, length, end, QLatin1String("</td"))
         && !hasPrefix(doc, length, end, QLatin1String("<td"))
         && !hasPrefix(doc, length, end, QLatin1String("</th"))
         && !hasPrefix(doc, length, end, QLatin1String("<th"))
         && !hasPrefix(doc, length, end, QLatin1String("<td"))
         && !hasPrefix(doc, length, end, QLatin1String("</tr"))
         && !hasPrefix(doc, length, end, QLatin1String("<tr"))
         && !hasPrefix(doc, length, end, QLatin1String("</table"))) {

  …
}

这里对"<td"做了两次check。

八、总是True或False的条件

[#1] Shareaza项目 – char类型的值范围

void CRemote::Output(LPCTSTR pszName)
{

  …
  CHAR* pBytes = new CHAR[ nBytes ];
  hFile.Read( pBytes, nBytes );
  …
  if ( nBytes > 3 && pBytes[0] == 0xEF &&
             pBytes[1] == 0xBB && pBytes[2] == 0xBF )
  {
    pBytes += 3;
    nBytes -= 3;
    bBOM = true;
  }
  …
}

表达式"pBytes[0] == 0xEF"总是False。char类型的值范围是-128~127 < 0xEF,因此这个表达式总是False,导致整个if condition总是为False,与预期逻辑不符。

[#3] VirtualDub项目 – 无符号类型总是>=0

typedef unsigned short wint_t;

void lexungetc(wint_t c) {
  if (c < 0)
    return;
   g_backstack.push_back(c);
}

c是unsigned short类型,永远不会小于0,也就是说if (c < 0)永远为False。

[#8] MySQL项目 – 条件错误

enum enum_mysql_timestamp_type
str_to_datetime(…)
{
  …
  else if (str[0] != ‘a’ || str[0] != 'A')
    continue; /* Not AM/PM */
  …
}

if (str[0] != ‘a’ || str[0] != 'A')这个条件永远为真。也许这块本意是想用&&。

九、代码漏洞

导致漏洞的代码错误实际上也都是笔误、不正确的条件以及不正确的数组操作等。但这里还是想将一些特定错误划归为一类,因为入侵者可以利用这些错误来攻击你的代码,获取其利益。

[#1] Ultimate TCP/IP项目 – 空字符串的错误检查

char *CUT_CramMd5::GetClientResponse(LPCSTR ServerChallenge)
{
  …
  if (m_szPassword != NULL)
  {
    …
    if (m_szPassword != '\0')
    {
  …
}

第二个if condition check意图检查m_szPassword是否为空字符串,但却错误的将指针与'\0'进行比较,正确的代码应该是这样的:

if (*m_szPassword != '\0')

[#2] Chromium项目 – NULL指针的处理

bool ChromeFrameNPAPI::Invoke(…)
{
  ChromeFrameNPAPI* plugin_instance =
    ChromeFrameInstanceFromNPObject(header);
  if (!plugin_instance &&
      (plugin_instance->automation_client_.get()))
    return false;
  …
}   

一旦plugin_instance为NULL,!plugin_instance为True,代码对&&后面的子条件求值,引用plugin_instance将导致程序崩溃。正确的做法应该是:

if (plugin_instance &&
        (plugin_instance->automation_client_.get()))
  return false;

[#5] Apache httpd Server项目 – 不完整的缓冲区clear

#define MEMSET_BZERO(p,l)       memset((p), 0, (l))

void apr__SHA256_Final(…, SHA256_CTX* context) {
  …
  MEMSET_BZERO(context, sizeof(context));
  …
}

这个错误前面提到过,sizeof(context)只是指针的大小,将之改为sizeof(*context)就OK了。

[#7] PNG Library项目 – 意外的指针clear

png_size_t
png_check_keyword(png_structp png_ptr, png_charp key,
                    png_charpp new_key)
{
  …
  if (key_len > 79)
  {
    png_warning(png_ptr, "keyword length must be 1 – 79 characters");
    new_key[79] = '\0';
    key_len = 79;
  }
  …
}

new_key的类型为png_charpp,顾名思义,这是一个char**类型,但代码中new_key[79] = ‘\0′这句显然是要给某个char赋值,但new_key[n]得到的应该是一个地址,给一个地址赋值为’\0′显然是有误的。正确的写法应该是(*new_key)[79] = '\0'。

[#10] Miranda IM项目 – 保护没生效

void Append( PCXSTR pszSrc, int nLength )
{
  …
  UINT nOldLength = GetLength();
  if (nOldLength < 0)
  {
    // protects from underflow
    nOldLength = 0;
  }
  …
}

nOldLength椒UINT类型,其值永远不会小于0,因此if (nOldLength < 0)这行成了摆设。

[#12] Ultimate TCP/IP项目 – 不正确的循环结束条件

void CUT_StrMethods::RemoveSpaces(LPSTR szString) {
  …
  size_t loop, len = strlen(szString);
  // Remove the trailing spaces
  for(loop = (len-1); loop >= 0; loop–) {
    if(szString[loop] != ' ')
      break;
  }
  …
}

循环中的结束条件loop >= 0将永远为True,因为loop变量的类型是size_t是unsigned类型,永远不会小于0。

十、拷贝粘贴

和笔误不同,程序员们决不因该低估拷贝粘贴问题,这类问题发生了太多。程序员们花费了大量时间在这些问题的debug上。

[#1] Fennec Media Project项目 – 处理数组元素时出错

void* tag_write_setframe(char *tmem,
                         const char *tid, const string dstr)
{
  …
  if(lset)
  {
    fhead[11] = '\0';
    fhead[12] = '\0';
    fhead[13] = '\0';
    fhead[13] = '\0';
  }
  …
}

 

咋看一下,fhead[13]做了两次赋值,似乎没啥问题。但仔细想一下,最后那行程序员的原意极可能是想写fhead[14] = '\0'。问题就在这里了。

[#2] MySQL项目 – 处理数组元素时出错

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];
}

 

编写这类代码时,我猜绝大多数人会选择Copy-Paste,然后再逐行修改,问题就发生在修改过程中,上面的代码中当处理a[5] != b[5]时就忘记修改一个下标了:return (int) a[1] – (int) b[5];显然这里的正确代码应该是return (int) a[5] – (int) b[5]。

[#3] TortoiseSVN项目 文件名不正确

BOOL GetImageHlpVersion(DWORD &dwMS, DWORD &dwLS)
{
  return(GetInMemoryFileVersion(("DBGHELP.DLL"),
                                dwMS,               
                                dwLS)) ;            
}

BOOL GetDbgHelpVersion(DWORD &dwMS, DWORD &dwLS)
{
  return(GetInMemoryFileVersion(("DBGHELP.DLL"),
                                dwMS,                           
                                dwLS)) ;                        
}

GetImageHlpVersion和GetDbgHelpVersion都使用了"DBGHELP.DLL"文件,显然GetImageHlpVersion写错文件名了。应该用"IMAGEHLP.DLL"就对了。

[#4] Clang项目 – 等同的函数体

MapTy PerPtrTopDown;
MapTy PerPtrBottomUp;

void clearBottomUpPointers() {
  PerPtrTopDown.clear();
}

void clearTopDownPointers() {
  PerPtrTopDown.clear();
}

我们看到虽然两个函数名不同,但是函数体的内容是相同的,显然又是copy-paste惹的祸。做如下修改即可:

void clearBottomUpPointers() {
  PerPtrBottomUp.clear();
}

 

十一、Null指针的校验迟了

这里的“迟了”的含义是先使用指针,然后再校验指针是否为NULL。

[#1] Quake-III-Arena项目 – 校验迟了

void Item_Paint(itemDef_t *item) {
  vec4_t red;
  menuDef_t *parent = (menuDef_t*)item->parent;
  red[0] = red[3] = 1;
  red[1] = red[2] = 0;
  if (item == NULL) {
    return;
  }
  …
}

 

在校验item是否为NULL前已经使用过item了,一旦item真的为NULL,那程序必然崩溃。

十二、其他杂项

[#1] Image Processing 项目 – 八进制数

inline
void elxLuminocity(const PixelRGBus& iPixel,
                     LuminanceCell< PixelRGBus >& oCell)
{
  oCell._luminance = uint16(0.2220f*iPixel._red +
                            0.7067f*iPixel._blue + 0.0713f*iPixel._green);
  oCell._pixel = iPixel;
}

inline
void elxLuminocity(const PixelRGBi& iPixel,
                     LuminanceCell< PixelRGBi >& oCell)
{
  oCell._luminance = 2220*iPixel._red +
    7067*iPixel._blue + 0713*iPixel._green;
  oCell._pixel = iPixel;
}

第二个函数,程序员原意是使用713这个十进制整数,但0713 != 713,在C中,0713是八进制的表示法,Compiler会认为这是个八进制数。

[#2] IPP Sample工程 – 一个变量用于两个loop中

JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void)
{
  …
  for(c = 0; c < m_scan_ncomps; c++)
  {
    block = m_block_buffer + (DCTSIZE2*m_nblock*(j+(i*m_numxMCU)));

    // skip any relevant components
    for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++)
    {
      block += (DCTSIZE2*m_ccomp[c][/c][/c].m_nblocks);
    }
  …
}

变量c用在了两个loop中,这会导致只有部分数据被处理,或外部循环中止。

[#3] Notepad++项目 – 怪异的条件表达式

int Notepad_plus::getHtmlXmlEncoding(….) const
{
  …
  if (langT != L_XML && langT != L_HTML && langT == L_PHP)
    return -1;
  …
}

代码中的那行if条件等价于 if (langT == L_PHP),显然似乎不是作者原意,猜测正确的代码应该是这样的:

int Notepad_plus::getHtmlXmlEncoding(….) const
{
  …
  if (langT != L_XML && langT != L_HTML && langT != L_PHP)
    return -1;
  …
}

一种基于内存映射文件的系统运行数据提取方法

这是我无意中想到的一个方法,估计这个方法已经不是什么新鲜的东西了,很可能在类似的问题场景中早已经被使用了。不过这里还是要说说我的思维过程。

近期在学习一些Linux性能查看和分析方面的工具,比如top、iostat、vmstat以及sar等。在学习过程中我发现这些工具有个共同的特点,那就是她们采集的Linux运行数据都是从/proc下的文件中实时获取并计算而得出的。众所周知,/proc是Linux内核维护的一个虚拟文件系统,他允许用户在Linux运行时查看内核运行数据(用户可以像查看普通文件一样查看/proc下的目录和文件),甚至是运行时实时改变内核设置。Linux实现/proc的细节不是这里要关注的,吸引我的是Linux的这种提取运行数据的设计。这个设计将Linux运行数据的产生实现细节与第三方性能采集工具间的耦合最大化地解开,这样一来/proc就像是一种Linux的基础服务,为用户提供一种实时的运行数据信息。而用户侧的运行数据查看工具也可以根据用户的需求自由定制,因此有了top、iostat、vmstat、iotop、sar等关注点不同的工具。

好了,说完/proc后,再来说说我们的产品。用户长期以来一直在抱怨我们的产品监控和维护方面手段太过单一,产品就像是一个黑盒,没有提供一种自我运行观察的能力,让客户看不清阿看不清,用户无法实时获取当前某个节点上的业务运行状况,无法采集到这些业务运行的实时基础数据,这的确是我们长期以来的短板(以前这块受重视度也的确不足)。虽然这两年我们在改善运维手段方面的投入已经加大,并收到一些显著的效果,但方案都是集中的,且相对重量级的,不那么敏捷灵活 – 在单节点上依旧无法简单地获取该节点的运行数据。

结合/proc的设计以及我们所遇到的问题,我有了一个大胆的想法:是否可以给我们的业务系统也加上一种类似Linux /proc这样的可提供基础运行数据的服务能力呢?于是就有了下面的解决方法。

Linux /proc下面的数据文件是Linux Kernel维护的,并允许用户层的进程实时查看和配置数据。而对于我们的产品而言,提供基础数据的产品实例与提取基础数据的第三方程序是两个独立的用户level的进程,显然我们需要找到一种让这两个进程实时通信、低耦合的且性能代价极低的方法。

我首先想到的是文件,这似乎和/proc的方式一样。你查看一下sysstat源码会发现,像iostat、sar等工具都是用fopen以"r"方式打开/proc/下的各种stat文件,匹配和读取指标项后再统计的。但在User层,两个无亲缘关系进程共同操作一个文件 – 一个读,一个写,the file position indicator是很难控制的,可能涉及文件锁(flock/fcntl),还要考虑使用的库函数是否是带缓冲的(fread/fgets都是带缓冲 的,不能用),写端需要及时fsync/fflush。总而言之,这么做是甚为自讨没趣的,会给两个程序的实现都带来很大的复杂性以及各种“坑”的。

那用named fifo如何呢?一但用named fifo,这两个进程就会产生启动依赖,如果一端没有启动,另一端会一直阻塞;而且通过fifo传递多种业务数据还可能存在打包和解包的过程,实现起来复杂的很。这显然是耦合十分严重的糟糕方案。

两个进程既要有共同的识别目标,就像/proc/cpuinfo这样的已知路径,一个进程还要能及时地得到另外一个进程运行时的数据,我们不妨尝试一下内存文件映射这个方案:运行数据提供的进程映射一个已知目标文件,比如perf/xxstat,然后在映射后的地址上创建和更新指标数据。比如我们建立一个整型数组,数组的每个元素都代表一种运行指标;而运行数据提取进程同样映射该文件,并在映射后获得数组中的各个元素值。下面是一个示例程序:

/* producer */
int
main()
{
    FILE *fp = NULL;

    errno = 0;
    fp = fopen(STAT_FILE, "w+");
    if (fp == NULL) {
        printf("can not create stat file , err = %d\n", errno);
        return -1;
    }

    errno = 0;
    long size = sysconf(_SC_PAGESIZE);
    if (ftruncate(fileno(fp), size) != 0) {
        printf("can not set stat file size, err = %d\n", errno);
        fclose(fp);
        return -1;
    }

    errno = 0;
    char *p = NULL;
    p = mmap(NULL, size, PROT_WRITE|PROT_READ, MAP_SHARED, fileno(fp), 0);
    if (p == MAP_FAILED) {
        printf("can not mmap file, error = %d\n", errno);
        fclose(fp);
        return -1;
    }

    errno = 0;
    if (fclose(fp) != 0) {
        printf("can not close file, error = %d\n", errno);
        return -1;
    }

    /* round up to 8 */
    while((int)p % 8 != 0) {
        p++;
    }

    long long *q = (long long*)p;
    q[0] = 1;
    q[1] = 1000;
    q[2] = 10000;
    q[3] = 100000;

    while(1) {
        q[0] += 1;
        q[1] += 10;
        q[2] += 100;
        q[3] += 1000;
        usleep(200);
    }

    return 0;
}

该producer程序首先尝试以"w+"方式打开xxstat文件,并设置文件的大小,然后调用mmap做内存文件映射,理论上来说mmap成功时返回的地址一定是按该平台下最严格内存系数对齐的地址,但这里为了安全起见,又做了一次内存地址的圆整。producer以映射的地址为首地址,建立了一个包含四个元素的、每个元素大小为8字节的整型数组,其中每个元素模拟一个运行指标。在while(1)循环中,producer模拟更新这四个指标数据。

下面是提取producer运行数据的例子程序,其映射过程与producer类似,这里就不贴出完整代码了,完整代码可在这里下载。

/* reader.c */

int
main()
{
    FILE *fp = NULL;
    … …

    char *p = NULL;
    p = mmap(NULL, size, PROT_READ,
             MAP_SHARED, fileno(fp), 0);
    if (p == MAP_FAILED) {
        printf("can not mmap file, error = %d\n", errno);
        fclose(fp);
        return -1;
    }

    … …

    long long *q = (long long*)p;

    while(1) {
        printf("%lld\t\t%lld\t\t%lld\t\t%lld\n", q[0], q[1], q[2], q[3]);
        sleep(1);
    }

    return 0;
}

在producer执行一段时间后,我们可以用reader去提取producer的实时运行数据了。

$ reader
2583        26820        268200        2682000
5793        58920        589200        5892000
9142        92410        924100        9241000
12431        125300        1253000        12530000
15586        156850        1568500        15685000
… …

需要注意的是两个进程映射的虽然是同一个文件,但各自进程空间映射的地址是不同的。如果在指标里存储地址数据,那另外一个进程在访问该地址时必然会出现问题。

在这个方案中,由于两个进程是读写同一块内存(虽然在各自进程空间的地址是不同的),因此数据是实时的。但由于两个进程间并没有任何同步机制,可能会产生误差,就好比一个进程中的两个线程对进程中某块地址空间一读一写这种情况一样。不过对于我们这种场景,这个问题是一般是可以被容忍和接受的,毕竟我们通过运行数据只是想了解一种运行趋势而已。如果producer中存在多个有亲缘关系的子进程或多线程要同时更新基础运行数据,那势必是要用锁或其他原子操作做数据操作的同步的。另外我们用的是内存映射具名的文件,OS会定期将数据刷到磁盘上,不过这个消耗对于小文件来说,对整体性能影响可忽略不计。

一旦业务系统具备了提供基础运行数据的能力,我们就可以根据我们的需求按照数据的格式打造我们所需要的各类数据提取和分析工具了。如果需要长期记录业务系统的运行情况,我们也可以实现类似sar这样的工具,以在后台定期对系统的运行数据进行记录,并提供历史查询等相关功能。

这种基于内存映射文件的方法还有一个好处,那就是我们可以用任何支持mmap调用的编程语言来实现数据提取工具,而不一定非得用C/C++这种原生适配Linux API的语言。

如果你觉得这种方案可行,那后续的重点就是基础运行数据的设计问题了。罗马不是一天建成的,/proc下的基础数据也不是一天就设计到位的。在基础数据设计这方面也是需要有很多考虑的,比如是文本还是二进制,用什么类型数据,还可能需要考虑一些数据对齐问题等。当然这就不是本文的重点了,就不细说了。




这里是Tony Bai的个人Blog,欢迎访问、订阅和留言!订阅Feed请点击上面图片

如果您觉得这里的文章对您有帮助,请扫描上方二维码进行捐赠,加油后的Tony Bai将会为您呈现更多精彩的文章,谢谢!

如果您喜欢通过微信App浏览本站内容,可以扫描下方二维码,订阅本站官方微信订阅号“iamtonybai”;点击二维码,可直达本人官方微博主页^_^:



本站Powered by Digital Ocean VPS。

选择Digital Ocean VPS主机,即可获得10美元现金充值,可免费使用两个月哟!

著名主机提供商Linode 10$优惠码:linode10,在这里注册即可免费获得。

阿里云推荐码:1WFZ0V立享9折!

View Tony Bai's profile on LinkedIn


文章

评论

  • 正在加载...

分类

标签

归档











更多