Ken Raeburn <raeburn@raeburn.org> writes:

> On Aug 22, 2009, at 16:18, joakim@verona.se wrote:
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>> joakim@verona.se writes:
>>>
>>>> +  GCPRO6 (file_description, file_mime, file_encoding, rv,
>>>> absname, encoded_absname);
>>>
>>> That's too much.  You only need to protect variables used around
>>> calls
>>> that can GC.  Arguments to lisp functions are implicitly protected.
>>> For
>>> example, there are no function calls during the lifetime of absname.
>>> And encoded_absname is completely unused.
>>
>> It seems to me I only need to protect f, which I would do by GCPRO:ing
>> absname. Since this is aparently wrong, I will leave it like it is,
>> since it doesnt hurt to GCPRO too much. (?)
>
> If ENCODE_FILE returns a new lisp string object, you need to GCPRO
> that object, not absname.  After the call to ENCODE_FILE, absname is
> unused, so it won't need protection.  In fact, it looks to me like
> after that point, GC isn't possible, so I'm not sure anything needs
> GCPRO'tection here.
>

I seem to be having trouble with GCPRO. Now I've marked the places I
believe might gc in the code.

>>> report_file_error throws, so you leak a resource.
>>
>> Fixed I think.
>
> No, you're still trying to call magic_close after report_file_error
> returns, which it won't.

Maybe I sent the wrong patch revision last time, better now then?

> CHECK_STRING is executable code, and should be moved down after the
> variable declarations.

same here.


> It seems to be common -- I'm not sure if it's required, but I would
> conservatively assume so -- for any local variable GCPRO'd to get
> initialized before any possible call to garbage collection, so the
> precise(?) garbage collector won't be scanning random stack values and
> thinking they're lisp objects; if the initialization is unclear, often
> that means simply assigning Qnil just before or after the GCPRO call.
>
> Different GC marking strategies are used on different platforms, so
> the lack of an obvious problem on one platform doesn't mean the code
> will work okay on another.
>
> Ken
-- 
Joakim Verona