Subject: Re: failure of unzip and recent cygwin1.dll
From: "Pierre A. Humblet" <[email-masked]>
To: [email-masked]
Cc: [email-masked], [email-masked]
Date: Mon, 16 Feb 2004 22:24:02 -0500

At 10:09 PM 2/16/2004 -0500, Igor Pechtchanski wrote:
>On Mon, 16 Feb 2004, Pierre A. Humblet wrote:
>
>> At 01:20 PM 2/16/2004 -0500, Christopher Faylor wrote:
>> >On Mon, Feb 16, 2004 at 12:36:14PM -0500, Thomas L Roche wrote:
>> >>
>> >>No, I have discovered considerably more. Consequently my question is,
>> >>is the path_conv bad?
>> >
>> >What you are debugging is the consequences of cmalloc being NULL.  While
>> >that may illustrate that cygwin should recover more robustly from such a
>> >situation, it is not directly related to the problem at hand, namely,
>> >"Why is cmalloc returning NULL?"
>>
>> I noticed that a) Thomas' file names are unusually long and
>> b) path_conv::set_normalized_path calls cmalloc only for long paths.
>>
>> Thus I decided to check if the normalized path is correctly freed.
>> That would explain why cmalloc is returning NULL.
>> As far as I can see, it isn't freed, at least not all the time.
>> When running /bin/ls very_long_path I see 4 allocs and 2 frees.
>>
>> However I don't find an obvious bug and I don't have the time to pursue
>> this for the moment.
>>
>> Pierre
>
>If I read the code correctly, normalized_path has to be explicitly freed.
>One of the places normalized_path is freed is in conv_path_list_buf_size,
>and the cfree is followed by the following FIXME comment:
>
>	cfree (pc.normalized_path);
>	// FIXME - probably should be in a destructor but
>	// it's hard to justify a destructor for the few
>	// places where this is needed
>
>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.

Yep, I saw that. There are also two other cfree spots, the main one in
fhandler.cc
and an unusual one in syscalls.cc. Btw., it looks like normalized path is
already
set when alloc is called again. If so a destructor would not help. But that
overwriting may also be a normal side effect of fhandler_base::operator = .

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/