Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
From: "Dave Korn" <dave DOT korn AT artimi DOT com>
To: <cygwin AT cygwin DOT com>
Subject: RE: autossh crash with 20050314 and earlier cygwin1.dll [autossh maintainer please note]
Date: Wed, 16 Mar 2005 18:30:05 -0000
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
In-Reply-To:  <MPG.1ca23a5849180d8a9896d4@localhost>
Message-ID: <SERRANOHZomtvWs7uuq0000006e@SERRANO.CAM.ARTIMI.COM>
Note-from-DJ: This may be spam

----Original Message----
>From: Andrew Schulman
>Sent: 16 March 2005 17:44


> OK. (hangs head) I have to take the blame for this, because as it
> happens I contributed that bit of code.  Forgive my simplicity on this
> point, but it seemed like good memory heigene-- free up the memory that
> gethostbyname() allocated, once I was done with it.  Looking back now at
> the man page for gethostbyname() (in Linux, there's no Cygwin man page
> AFAIK), it seems unclear on this point:  the NOTES section says "The
> functions gethostbyname() and gethostbyaddr() *may* return pointers to
> static data, which may be overwritten by later calls." (emphasis added)
> 
> This may have been a dumb mistake, or not.  Obviously if the memory is
> static, I shouldn't free() it, but the docs are unclear about whether it
> is or not.  Should I infer that I should not free() memory allocated by
> a function that I call unless the man page specifically says to?  Or
> just that it's best not to free() it if it might be static?  


  You *MUST* not pass any value to the free (...) function that was not
previously returned by one of the *alloc (...) functions.  That is an
absolute requirement to avoid undefined behaviour.  If *you* don't know
where it comes from, *you* must not free it.

  However, there is a second issue here.  Even if it was malloced, and
freeing it wouldn't cause a crash, that _still_ doesn't mean you should or
could free it, even assuming your code was somehow able to test the value
returned at runtime to know whether it pointed into the heap or not.  The
thing is, you got given a pointer to this result, but for all you know the
library kept a copy of that pointer before it returned, and may be planning
to use it again.

  If the library allocates it, it is up to the library whether or not to
free it; for all you know, the C runtime mallocs a buffer at startup, uses
it throughout the life of the program for repeated calls to gethostbyname,
and free()s it at program exit time.  In that case, you'd be causing the
library to write into free'd memory, which is bad.


  So to sum up the generic-software-engineering-technique issues:

1) Yes, memory hygiene IS a good thing, and freeing all your memory rather
than waiting for it to be deallocated when you exit is a good thing.  (And
some programs don't exit, or are expected to run continuously for
days/weeks/months/years between restarts.)

2)  However, this particular issue is not a question of memory hygiene, but
of resource ownership and responsibility for deallocation.

  Generally speaking, it is whoever mallocs memory that has the
responsibility for freeing it.  If your app mallocs it, your app should free
it; if the library mallocs it, the library should free it.

  In some cases, the ownership of a bit of memory (or any other resource)
may be transferred; in those cases, the responsibility for deallocation is
transferred along with the ownership.

  But that does not happen without a clear statement in the interface /
documentation.  A function that returns a pointer to some data in memory is
giving you just that, and no more: a pointer to some data in memory.  Only
if the specification of the function explicitly states that the caller gets
ownership of the resource should your code assume it has any rights over it
whatsover.  If the interface doesn't say so, then you should assume nothing
- not just that you can't free (...) it, but you should also assume that you
aren't allowed to overwrite the memory block or change its contents or do
anything other than examine and/or copy it.


  Really, the gethostbyname function should have been defined to return a
(const struct hostent *) in the first place, in order to make clear to you
that it's not yours and you mustn't do anything to it, but that all happened
a long time ago back in the early days of K&R C and Berkeley sockets, and
it's just too late to change now.

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/