在C ++成员函数中“if(!this)”有多糟?

如果我遇到旧的代码, if (!this) return; 在一个应用程序,这是多么严重的风险? 这是一个危险的定时炸弹,需要立即在应用程序范围内进行search和销毁,还是更像是一种可以安静地放置的代码味?

当然,我不打算编写这样的代码。 相反,我最近在我们的应用程序的许多部分使用的旧核心库中发现了一些东西。

想象一下CLookupThingy类有一个非虚拟的 CThingy *CLookupThingy::Lookup( name )成员函数。 很显然,在那些牛仔时代,其中一位程序员遇到了很多崩溃,从函数传递NULL CLookupThingy * s,而不是修复数百个调用站点,他悄悄地修复了Lookup():

 CThingy *CLookupThingy::Lookup( name ) { if (!this) { return NULL; } // else do the lookup code... } // now the above can be used like CLookupThingy *GetLookup() { if (notReady()) return NULL; // else etc... } CThingy *pFoo = GetLookup()->Lookup( "foo" ); // will set pFoo to NULL without crashing 

本周早些时候我发现了这个gem,但是现在我是否应该解决这个问题呢? 这是我们所有应用程序使用的核心库。 其中有几个应用程序已经被运送给数百万客户,而且似乎工作正常。 该代码没有崩溃或其他错误。 在查找函数中删除if !this将意味着修复数千个可能传递NULL的调用站点; 不可避免的会有一些错过,引入新的错误,在未来一年的发展中随机出现。

所以我倾向于放弃它,除非绝对必要。

鉴于这是技术上未定义的行为, if (!this)在实践中有多危险? 是否值得劳力几周来修复,还是MSVC和GCC可以安全地返回?

我们的应用程序编译MSVC和GCC,并在Windows,Ubuntu和MacOS上运行。 移植到其他平台是无关紧要的。 有问题的function保证永远不会是虚拟的。

编辑:我正在寻找的那种客观答案是类似的

  • “当前版本的MSVC和GCC使用了一个ABI,其中非虚拟成员真的是静态的,带有一个隐含的'this'参数;因此,即使'this'为NULL,它们也可以安全地分支到函数中。
  • “即将到来的GCC版本将改变ABI,使得即使非虚拟function也需要从类指针中加载分支目标”或
  • “目前的GCC 4.5有一个ABI不一致的地方,有时它将非虚拟成员编译成带有隐式参数的直接分支,有时也作为类偏移量函数指针。

前者意味着代码很臭,但不会破坏; 第二个是编译器升级后要testing的东西; 后者需要立即采取行动,即使成本很高。

显然这是一个潜在的bug,但现在我只关心如何减轻我们特定编译器的风险。

我会放弃它。 这可能是一个老式版本的SafeNavigationOperator的故意select。 正如你所说,在新的代码中,我不会推荐它,但是对于现有的代码,我会放弃它。 如果你最终修改它,我会确保所有的调用都被testing覆盖。

编辑以添加:您可以select仅通过以下代码在代码的debugging版本中将其删除:

 CThingy *CLookupThingy::Lookup( name ) { #if !defined(DEBUG) if (!this) { return NULL; } #endif // else do the lookup code... } 

因此,它不会破坏生产代码上的任何内容,同时给你一个机会在debugging模式下进行testing。

像所有未定义的行为

 if (!this) {  return NULL; } 

一个等待下去的炸弹。 如果它与你现在的编译器一起工作,你真幸运,有点不吉利!

相同编译器的下一个版本可能更具侵略性,并将其视为死代码。 因为this永远不能为空,代码可以“安全地”被删除。

我觉得如果你把它移走,会更好!

如果你有很多GetLookup函数返回NULL,那么你最好修复使用NULL指针调用方法的代码。 首先,replace

 if (!this) return NULL; 

 if (!this) { // TODO(Crashworks): Replace this case with an assertion on July, 2012, once all callers are fixed. printf("Please mail the following stack trace to myemailaddress. Thanks!"); print_stacktrace(); return NULL; } 

现在,继续你的其他工作,但修复这些当他们滚入。replace:

 GetLookup(x)->Lookup(y)... 

 convert_to_proxy(GetLookup(x))->Lookup(y)... 

其中conver_to_proxy确实返回的指针不变,除非它是NULL,在这种情况下,它会返回一个FailedLookupObject在我的其他答案。

在大多数编译器中它可能不会崩溃,因为非虚函数通常是内联的或者被翻译成以“this”作为参数的非成员函数。 但是,该标准特别指出,在对象的生命周期之外调用非静态成员函数是未定义的,并且对象的生命周期被定义为当对象的内存被分配并且构造器已经完成时开始非平凡的初始化。

标准只是在构造或销毁过程中对对象本身所做的调用规定了一个例外,但即使如此,也必须小心,因为虚拟调用的行为可能与对象的生命周期中的行为不同。

TL:DR:即使需要很长时间清理所有呼叫点,我也会用火把它杀死。

编译器的未来版本可能会在正式未定义行为的情况下更积极地进行优化。 我不担心现有的部署(你知道编译器实际执行的行为),但是它应该在源代码中修复,以防你使用不同的编译器或不同的版本。

这就是所谓的“一个聪明和丑陋的黑客”。 注意:聪明!=明智。

find所有没有任何重构工具的调用网站应该很容易; 以某种方式打破GetLookup(),所以它不会编译(例如更改签名),以便您可以静态识别错误使用。 然后添加一个名为DoLookup()的函数,它可以完成所有这些黑客正在做的事情。

在这种情况下,我build议从成员函数中删除NULL检查,并创build一个非成员函数

 CThingy* SafeLookup(CLookupThing *lookupThing) { if (lookupThing == NULL) { return NULL; } else { return lookupThing->Lookup(); } } 

然后,应该很容易find对Lookup成员函数的每个调用,并将其replace为安全的非成员函数。

如果今天有些事情让你感到困扰,那么一年之后它会打扰你。 正如你所指出的那样,改变它几乎肯定会引入一些错误 – 但是你可以先保留return NULLfunction,添加一些日志logging,让它在野外运行几个星期,然后发现它有多less次受到打击?

今天可以通过返回失败的查找对象来安全地解决这个问题

 class CLookupThingy: public Interface { // ... } class CFailedLookupThingy: public Interface { public: CThingy* Lookup(string const& name) { return NULL; } operator bool() const { return false; } // So that GetLookup() can be tested in a condition. } failed_lookup; Interface *GetLookup() { if (notReady()) return &failed_lookup; // else etc... } 

此代码仍然有效:

 CThingy *pFoo = GetLookup()->Lookup( "foo" ); // will set pFoo to NULL without crashing 

只有当你对规范的措辞迂腐时,这才是“滴答作响的炸弹”。 然而,无论如何,这是一个可怕的,不明智的做法,因为它掩盖了一个程序错误。 仅仅因为这个原因,我会删除它,即使这意味着很多的工作。 这不是一个直接的(甚至是中期的)风险,但它不是一个好的方法。

这种错误隐藏行为确实不是你想要依赖的东西。 想象一下,你依赖于这种行为(也就是说,我的对象是否有效,无论如何都是无关紧要的 ),然后,由于某种危险,编译器在特定情况下优化if语句,因为它可以certificatethis是不是空指针。 这是一个合理的优化,不仅适用于某些假设的未来编译器,而且适用于非常真实的当前编译器。
但是,当然,由于你的程序不是完整的,所以在某些时候,你会在20个angular落传递一个null。 砰,你死了。
不可否认,这是非常好的,不会发生的,但是你不可能100%确定它仍然不可能发生。

请注意,当我喊出“移除!”时,并不意味着必须立即或在一次大规模的人力操作中将其全部移除。 您可以在遇到这些问题时逐个删除这些检查(当您在同一文件中更改某些内容时,避免重新编译),或者只是对文本进行search(最好是在高度使用的函数中),然后删除它们。

由于您使用的是GCC,因此您可能会在__builtin_return_address__builtin_return_address ,这可能会帮助您删除这些无需大量人力的检查,并完全中断整个工作stream程并使应用程序完全无法使用。
删除支票之前,将其修改为输出呼叫者的地址, addr2line会告诉你在你的来源的位置。 这样,您应该能够快速识别应用程序中存在错误行为的所有位置,以便修复这些位置。

所以,而不是

 if(!this) return 0; 

每次更改一个位置,如下所示:

 if(!this) { __builtin_printf("!!! %p\n", __builtin_return_address(0)); return 0; } 

这可以让你识别这个更改的无效呼叫站点,同时让程序“按预期工作”(如果需要,你也可以查询呼叫者的呼叫者)。 修复每一个不良行为的地点,一个接一个。 该scheme将仍然“正常”工作。
一旦没有更多的地址出现,全部删除检查。 如果你运气不好(因为它在testing中没有显示),你可能仍然必须修复其中一个或另一个崩溃,但这应该是非常罕见的事情发生。 无论如何,都应该防止你的同事向你大喊大叫。
每周取消一次或两次支票,最终不会遗漏。 同时,生活还在继续,没有人注意到你在做什么。

TL; DR
至于“当前版本的GCC”,对于非虚函数你没有问题,但是当然谁也不知道未来版本可能会做什么。 我认为,未来的版本不太可能会导致代码崩溃。 不less现有的项目都有这种检查(我记得我们有几百个Code :: Blocks代码完成了,不要问我为什么)。 编译器制造商可能不想让几十个/几百个主要的项目维护人员故意不高兴,只是为了certificate一个观点。
另外,请考虑最后一段(“从逻辑的angular度来看”)。 即使这个检查会崩溃,并与未来的编译器烧伤,它会崩溃,并烧伤无论如何。

if(!this) return; 语句是有用的,因为this不能在一个格式良好的程序中成为一个空指针(这意味着你在一个空指针上调用了一个成员函数)。 当然,这并不意味着它不可能发生。 但是在这种情况下,程序应该会崩溃或者断言。 这种scheme在任何情况下都不应该默默地继续下去。
另一方面,完全有可能在一个无效的对象上调用一个成员函数,而这个对象不是null 。 检查this是否是空指针显然不能捕捉到这种情况,但它是完全相同的UB。 所以,除了隐藏错误的行为之外,这个检查也只能检测一半有问题的情况。

如果你正在通过speficication的措辞,使用(包括检查它是否是一个空指针)是未定义的行为。 就严格来说,这是一个“定时炸弹”。 但是,我从理论上和逻辑上都不合理地认为这是一个问题。

  • 实际angular度来看,只要不解除引用读取无效的指针并不重要。 是的,严格来说,这是不允许的。 是的,理论上有人可能会build立一个CPU,当你加载它们时会检查无效的指针,并且出错。 唉,如果你是真实的,情况并非如此。
  • 逻辑的angular度来看,假设支票将会爆炸,但这仍然不会发生。 为了执行这个语句,必须调用成员函数,并且(虚拟或不内联或不内联)使用一个无效的this ,它在函数体内可用。 如果非法使用this ,那另一个也会。 因此,检查已经过时,因为程序已经崩溃了。

nb:这个检查与“安全删除成语”非常相似,它在删除它之后(使用macros或模板化的safe_delete函数)设置指向nullptr的指针。 据推测,这是“安全的”,因为它不会崩溃两次删除相同的指针。 有些人甚至添加了冗余if(!ptr) delete ptr;
如你所知, operator delete保证是空指针上的空operator delete 。 也就是说,通过设置一个指向空指针的指针,你已经成功地消除了检测双重删除的唯一机会(这是一个需要修复的程序错误!)。 它没有任何“更安全”,但却隐藏了不正确的程序行为。 如果你删除了一个对象两次,程序会崩溃。
你应该保留一个被删除的指针,或者,如果你坚持篡改,把它设置成一个非空的无效指针(比如一个特殊的“无效的”全局variables的地址,或者一个魔术值,如-1 – 但是,当它发生时,不应该试图欺骗和隐藏崩溃)。

我个人认为,你应该尽早失败,以提醒你的问题。 在这种情况下,我会毫不客气地删除if(!this)我能find的每一个事件。