Mail Archives: cygwin/2004/02/17/14:36:58
On Tue, Feb 17, 2004 at 12:33:34PM -0500, Christopher Faylor wrote:
> On Tue, Feb 17, 2004 at 11:09:39AM -0500, Christopher Faylor wrote:
> >>I believe a destructor would be cleaner, and the current code obviously
> >>misses at least one more place where this is needed. Unfortunately, with
> >>my copyright assignment in flux, I can't send in a patch. If noone fixes
> >>this by the time I can send patches, I'll try to send in a fix for this.
> >
> >It's not that simple. path_conv objects can persist, even over fork/exec.
> >So a simple destructor would be guaranteed to do the wrong thing.
>
> Actually maybe it is that simple. In my exhaustive testing (i.e., one
> program) it seems like a destructor works just fine. This is, I
> suppose, a tribute to the person who wrote most of the path handling
> code. I should have had more faith in his abilities.
>
> On reflection, another reason for not doing things this way was to avoid
> the cost of a destructor for all of the cases that path_conv was used
> when it wasn't really needed for the vast majority of cases. I've been
> trying to move more and more to using fhandler_* instead of path_conv
> and fhandler does free normalized_path. However, I obviously haven't
> done a 100% conversion so it's better to be safe.
I think I have figured it out.
stat_worker (and others like it) calls fh = build_fh_name()
which instantiates a path_conv, which may call cmalloc for the
normalized_path.
If everyting goes OK, the path_conv is copied into the handler
structure, in fh->set_name (pc).
There lies the rub: the normalized_path is cmalloced again.
That was unnecessary (before the destructor was added) because the
normalized path was not destroyed when build_fh_name() terminates.
As Chris points out, the original method (without destructor) is more
efficient (even more so without the unneeded cmalloc), but there is a
danger of leak.
However a normalized path is only created when the PC_POSIX flag is
given, which happens only in 3 spots. AFAICS the path_conv was
properly handled in these three cases.
Pierre
--
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/
- Raw text -