Opened 10 years ago

Closed 10 years ago

#98 closed defect (fixed)

review: fix to "auth server doesn't create IPv6 socket"

Reported by: jinmei Owned by: jinmei
Priority: high Milestone: 03. 1st Incremental Release
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Please review the attached fix.

The bug is trivial, and just fixing it isn't difficult, but I think there are more fundamental issues around this bug:

First, the semantics of -4 and -6 could confuse the code logic. -4 means "ipv4_only", but its negative sense can confuse the coder (me), and we could interpret it as "use ipv4". I made that bug before.

So, in the revised version, I tried to introduce new variables with more intuitive names: use_ipv4 and use_ipv6. But then there was another regression bug regarding the new variables. In general, having more variables opens up the possibility of more bugs.

Now, in the attached patch I try to "minimize" the problem by:

  • use positively named variables (use_ipv4 and use_ipv6)
  • use these variables only

These are not perfect solution in that we need to do something counterintuive:

case '4':

use_ipv6 = false;
break;

that is, set the "*6" variable in the case of '4'.

But this is propbably unsolvable because the semantics of -4 and -6 are inherently negative. So I'd consider this a best-current-practice fix.

Subtickets

Attachments (1)

46.diff (1.4 KB) - added by jinmei 10 years ago.

Download all attachments as: .zip

Change History (5)

Changed 10 years ago by jinmei

comment:1 Changed 10 years ago by jreed

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

comment:2 Changed 10 years ago by jreed

I tested the four possible ways. This works.

comment:3 Changed 10 years ago by jreed

  • Owner changed from jreed to jinmei

comment:4 Changed 10 years ago by jinmei

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

fix committed (r1460). closing.

Note: See TracTickets for help on using tickets.