Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#3861 closed enhancement (fixed)


Reported by: fdupont Owned by: fdupont
Priority: very high Milestone: Kea0.9.2-beta
Component: Unclassified Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by fdupont)

My proposal

I can see in some new code a constant member named INVALID_SOCKET. It
is a good name but in fact a too good name as it collides with at
least a very popular system (where it is defined to ~0 in the case
some codes strangely fail to compile :-).
I strongly suggest to change the name a bit, for instance for INVALID__SOCKET .
Note I believed we were already happy with the asio stuff which provides
a system independent asynchronous I/O system? It is not I don't like
the socket watcher mechanism (it is used by bind9 for instance) but
it is incompatible with non Unix systems (Windows has a nice but
different way to solve this problem, again look at bind9 code).

Stephen's answer:

I think I'd suggest a name such as SOCKET_NOT_VALID. I know that this
sounds a bit like splitting hairs, but unless we have an established
naming convention using double underscores, a "one off" use of a name
containing them is likely to lead to confusion. (Especially if we do
release Kea on the system where INVALID_SOCKET is already defined.)


Change History (9)

comment:1 Changed 5 years ago by fdupont

  • Description modified (diff)

comment:2 Changed 5 years ago by fdupont

  • Description modified (diff)

comment:3 Changed 5 years ago by fdupont

  • Description modified (diff)

comment:4 Changed 5 years ago by fdupont

Ready for review!!!

comment:5 Changed 5 years ago by fdupont

  • Owner set to UnAssigned
  • Status changed from new to reviewing

comment:6 Changed 5 years ago by stephen

  • Owner changed from UnAssigned to fdupont

Reviewed commit 1f7b2c89e220f9a6a228692c473a359887d58c35

I think this needs a ChangeLog entry. Something like:

to avoid conflicting with a constant of that name defined on some
operating systems.

All OK, please merge.

comment:7 Changed 5 years ago by fdupont

Merged. Closing...

comment:8 Changed 5 years ago by fdupont

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

comment:9 Changed 5 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea0.9.2
Note: See TracTickets for help on using tickets.