[ wxwindows-Patches-1626716 ] remove flawed deepCopyHostent/deepCopyServent implementation

[SourceForge.net]

Patches item #1626716, was opened at 2007-01-03 08:32
Message generated for change (Tracker Item Submitted) made by Item Submitter
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1626716&group_id=9863

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Socket
Group: bug fix
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Denis Lukianov (voxelsoft)
Assigned to: Nobody/Anonymous (nobody)
Summary: remove flawed deepCopyHostent/deepCopyServent implementation

Initial Comment:
The implementations of deepCopyHostent and deepCopyServent in src/unix/gsocket.cpp do not correctly perform a deep copy of a given hostent.

deepCopyHostent copies the content of the old hostent structure into a new hostent structure. It then proceeds to copy the address and alias list. However, the pointers to those lists are not updated, so the new list (despite having new content) has a member h_addr_list that points directly into the old list. This is a shallow deep copy at best.

Moreover, deepCopyHostent modifies the old structure in the process of copying because it uses the old h_addr_list to update pointers. This is a severely bad thing to do, because the old hostent is statically allocated by the operating system. The problem with this is not apparent on most operating systems. But on OSX, for example, you will invariably get the following system error on the subsequent gethostname call:

malloc: ***  Deallocation of a pointer not malloced: 0xbfffe55b; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug

This is also easily demonstrated in a clean application that does not include the wx libraries, and only sufficient code to run the following sequence:
gethostname (given hostname must be valid)
deepCopyHostent (implementation taken from wx)
gethostname

A proper implementation would need to store all pointers in the new buffer or resort to memory allocation (and in the buffer case, to take care of pointer alignment). It should not modify the old structure. I have constructed such an implementation, and it works out to be very long and difficult to read.

Upon closer inspection, the functions are not at all required. There is no need to perform a copy of the structure as it is not required at any other point except the name resolution.

Same arguments apply to deepCopyServent.

This patch removes functions deepCopyHostent and deepCopyServent, using the original hostent structure directly.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1626716&group_id=9863