Bug 34605 – check-webkit-style incorrectly complains about NULL appearing in a comment

• Status: RESOLVED
• Resolution: INVALID
• Component: Tools / Tests

[Gustavo Noronha (kov)]

See https://bugs.webkit.org/show_bug.cgi?id=34602

check-webkit-style reported this:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/network/soup/DNSSoup.cpp:41:  Use 0 instead of NULL. 
[readability/null] [4]
Total errors found: 1

And this is the line it's complaining about:

 41     // We may get invalid hostnames, so NULL-check here.

[David Levin]

(In reply to comment #0)
> See https://bugs.webkit.org/show_bug.cgi?id=34602
> 
> check-webkit-style reported this:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/platform/network/soup/DNSSoup.cpp:41:  Use 0 instead of NULL. 
> [readability/null] [4]
> Total errors found: 1
> 
> And this is the line it's complaining about:
> 
>  41     // We may get invalid hostnames, so NULL-check here.

fwiw, this check was done this way on purpose. The check was carefully done to include both code and comments but not items in quotes (in code).

 Several experienced reviews have commented on NULL appearing in comments. As I understand it, the reason being that NULL isn't allowed in code so it doesn't make sense in comments. 

For example, the comment could easily have said
  // Check for invalid hostnames.
and seems to talk more about why than what (checking for a 0 value).

Another recent instance is this comment 
  if (hdc) { // Can this ever actually be NULL?
which could have easily been 
  if (hdc) { // Can hdc ever actually be 0?

[Alexey Proskuryakov]

Yes, I agree that this particular comment needn't mention null check at all. And we do prefer to talk about null or 0 when necessary.