Opened 7 years ago

Closed 7 years ago

#2357 closed defect (fixed)

Use of reserved identifiers

Reported by: vorner Owned by: muks
Priority: medium Milestone: Sprint-20121106
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

According to the C++ (and C) standard, everything that:

  • Begins with and underscore and then there's an upper case letter
  • Contains double underscore (two underscores following each other)

are reserved for the implementation of compiler.

What I understand is the authors of compilers are free to use such names for their crazy internal macros and functions as they wish and if they clash with our such names, it's our problem. In other words, such code using them has undefined behaviour.

I noticed we have some such names through the code (usually the #ifndef HEADER_NAME guards). We should replace them by something less dangerous.

Subtickets

Attachments (1)

fix-scaffolding.py (1.3 KB) - added by muks 7 years ago.
Tool to update the #ifndef scaffolding in header files

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20121106

comment:2 Changed 7 years ago by muks

  • Owner set to muks
  • Status changed from new to assigned

Picking

comment:3 Changed 7 years ago by muks

There are hundreds of such cases, so I wrote a tool to update the source code called fix-scaffolding.py. I'll attach it to this bug.

Changed 7 years ago by muks

Tool to update the #ifndef scaffolding in header files

comment:4 Changed 7 years ago by muks

  • Owner changed from muks to UnAssigned
  • Status changed from assigned to reviewing

Up for review

comment:5 Changed 7 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:6 follow-up: Changed 7 years ago by jelte

  • Owner changed from jelte to muks

what has been changed all looks good, and a quick clean build + tests succeeded.

I have pushed a few additional changes:

A doxygen comment in the message builder wasn't updated, for which i pushed a small fix. I also updated the rdataclass.h generator (which included <name> too).

(both of these in commit 1)

In second commit i just cleaned up a few #endif comments; looks like the script didn't hit them because they didn't match the original guard name in the first place.

With that AFAICT they are all gone; except for PyInit__dns in acl wrappers, but i think the file naming (which itself starts with an underscore) causes that name to need 2.

So your changes are OK, if mine are too (by all means check twice), this can be merged :)

comment:7 in reply to: ↑ 6 Changed 7 years ago by muks

Replying to jelte:

So your changes are OK, if mine are too (by all means check twice), this can be merged :)

Changes all look OK to me. I'll merge them soon.

comment:8 Changed 7 years ago by muks

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged to master in commit dc3884d6b3e3f80144d57a6edc0223d97f5a46af:

* 195617a [2357] Wrap comments above 80 char
* 0eb033a [2357] clean up a few #endif comments
* 5744cc1 [2357] additional __ fixes
* fa6cef4 [2357] Rename macro to avoid conflict with another one
* e710685 [2357] Manually fix the odd ends that were left
* 059a2a1 [2357] Update scaffolding using fix-scaffolding.py tool
* 6d13586 [2357] Update the scaffolding which message compiler generates

Resolving as fixed. Thank you for the review and further patches Jelte.

Note: See TracTickets for help on using tickets.