Opened 7 years ago

Closed 6 years ago

#2112 closed enhancement (fixed)

confusing initialization in for loops

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

Description

Microsoft C++ compilers complain about this style:
(from src/lib/datasrc/tests, file factory_unittest.cc)

for (const char** config(configs); *config; ++config) {

obviously they don't correctly infer the type of "config" and take it as a function which can't be a left-value... IMHO it is better (and required until all Microsoft compilers are fixed) to use:

for (const char** config = configs; *config; ++config) {

BTW it seems all the files with this confusing style are in this directory.

Subtickets

Change History (10)

comment:1 Changed 7 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:2 Changed 7 years ago by jinmei

I agree with the proposal on the style, but I guess we should first
discuss whether we can have a consensus. Then it should go to the
style guideline page.

I also remember there was a quick discussion about this style matter
at bind10-dev (but I can't find a link).

comment:3 Changed 7 years ago by fdupont

Yes, there were complains about this kind of initialization in fd.cc. But here it is not a problem of style but a work around for a MSVC bug.

comment:4 Changed 7 years ago by shane

  • Estimated Difficulty changed from 0 to 3

comment:5 Changed 6 years ago by muks

  • Milestone set to Sprint-20131015
  • Owner set to UnAssigned
  • Status changed from new to reviewing

Up for review.

The case in factory_unittest.cc mentioned in the description is no longer present. The branch fixes all the cases I could find inside src/lib/datasrc/.

comment:6 Changed 6 years ago by muks

No ChangeLog is necessary. I don't think anyone apart from Francis builds this with VC++ yet. :)

comment:7 Changed 6 years ago by fdupont

BTW I gave up about compiling bind10 with MSVC. I am still on the crypto to avoid what I called the crypto library disaster in my recent blog but for WIN32 bind10 code is today far too Linux centric... I hope we'll get a C++ compiler/runtime as good as Visual Studio C++ for finding bugs (g++/libstdc++ is very poor for this).

comment:8 Changed 6 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:9 Changed 6 years ago by stephen

  • Owner changed from stephen to muks

Reviewed commit 38d7d64c7257ccb3cb7e29d76bfeeb64beda8cc0

I agree that no ChangeLog is necessary.

All OK, please merge.

comment:10 Changed 6 years ago by muks

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

Merged to master in commit fb631319f355d4807781090ac62f9db43a287485:

* 38d7d64 [2112] Fix initialization in for loops (for Microsoft C++)

Resolving as fixed. Thank you for the review Stephen.

Note: See TracTickets for help on using tickets.