Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#3837 closed enhancement (complete)

setenv() portability

Reported by: fdupont Owned by: fdupont
Priority: low 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

setenv() doesn't exist in WIN32 environment but can be replaced by _putenv_s() which does the same thing (modulo it doesn't provide the (unused) option to not overwrite an existing environment variable).
There is again a choice:

  • replace setenv() where it is used with a #ifndef _WIN32
  • define setenv() in the WIN32 config.h

IMHO the first is better because more visible. Of course the same argument can be used to prefer the second.

Subtickets

Change History (9)

comment:1 Changed 5 years ago by fdupont

Putting under temporary review to get comments about the choice.

comment:2 follow-up: Changed 5 years ago by marcin

I am strongly suggesting the second because it will be the change in the single place, rather than multiple.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 5 years ago by fdupont

Replying to marcin:

I am strongly suggesting the second because it will be the change in the single place, rather than multiple.

=> thanks Marcin!

If nobody objects I'll take the ticket and apply it. Note it should be not visible in master so won't be subject to a second round of review (vs the first solution).

comment:4 in reply to: ↑ 3 ; follow-up: Changed 5 years ago by marcin

Replying to fdupont:

Replying to marcin:

I am strongly suggesting the second because it will be the change in the single place, rather than multiple.

=> thanks Marcin!

If nobody objects I'll take the ticket and apply it. Note it should be not visible in master so won't be subject to a second round of review (vs the first solution).

The actual code should be reviewed before you commit it, though.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 5 years ago by fdupont

Replying to marcin:

If nobody objects I'll take the ticket and apply it. Note it should be not visible in master so won't be subject to a second round of review (vs the first solution).

The actual code should be reviewed before you commit it, though.

=> I was not very clear: the change will be in the win32 branch and the master branch will stay unchanged so you should have nothing to review (BTW if you'd like I can put the whole win32 stuff under review if you want, there is already a ticket for this, but it will be more to mark an end of phase than for something else as all changes for the master, including potential changes like the first (and not adopted) solution of this ticket are in tickets and (to be) reviewed).

comment:6 in reply to: ↑ 5 Changed 5 years ago by marcin

Replying to fdupont:

Replying to marcin:

If nobody objects I'll take the ticket and apply it. Note it should be not visible in master so won't be subject to a second round of review (vs the first solution).

The actual code should be reviewed before you commit it, though.

=> I was not very clear: the change will be in the win32 branch and the master branch will stay unchanged so you should have nothing to review (BTW if you'd like I can put the whole win32 stuff under review if you want, there is already a ticket for this, but it will be more to mark an end of phase than for something else as all changes for the master, including potential changes like the first (and not adopted) solution of this ticket are in tickets and (to be) reviewed).

Oh, I see. There is a question if this should go through the trac ticketing system or rather be discussed on the mailing list?

comment:7 Changed 5 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:8 Changed 5 years ago by fdupont

  • Resolution set to complete
  • Status changed from accepted to closed

comment:9 Changed 5 years ago by tomek

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