On Sat, 7 May 2011 11:41:00 +0200, VN wrote:

> > FWIW, and considering that the current refcounting in Geeqie is
> > troublesome, you've got my blessing. :-) I'm all for a central list of
> > the current directory's file data objects, with added maintenance
> > structures on top of that.
> 
> What do you mean by _central_? There is ViewFile->list for each window,
> but then there are standalone files too.

Central = a global list for each view. With a manager that maintains the
list (including the refreshing of the directory contents if external tools
modify the dir), the parent/child relationships, the references to
additional data caches, and the freeing of the FileData objects. The
manager would have the ultimate decision power whether and when to free
memory and to remove something from the list.

> > Additionally, the FileData unref method is questionable already (not just
> > due to its fd->magick checking and special handling of child nodes), since
> > for example it does
> > 
> >     fd->ref--;
> > 
> >     if (fd->ref == 0)
> >     {
> >             /* decide whether to file_data_free() or return earlier */
> >     }
> >     return;
> > 
> > and hence doesn't cover the case that ref<0, and the responsibility to
> > drop ptrs to freed FileData from any lists or locals, is not within this
> > method.
> > 
> 
> file_data_free is called when the file and it's parent and children all have 
> zero ref count. Then it frees whole group.

Who ensures that it is called like that?
The current implementation bails out if these contraints aren't met,
but the refcount is decreased nevertheless, as that's the first thing the
unref method does.  The unref method so far results in several of its
assertions to abort Geeqie (even sidecarfiles==NULL has happened due to the
file grouping issues), and the bugs that mess with the FileData objects
are elsewhere in the code, not in the unref method.

> ref<0 should not happen. The only possible handling in unref function is to 
> write an error message and crash.
>
> There may be some missing ref/unref calls in the code but this is fixable. At 
> the moment I don't think we should change the infrastructure.

I'm more concerned about too many unref calls _and_ unref calls for
invalid FileData pointers. That users sometimes manage to crash Geeqie
with the fd->magick=0x12345678 assertion failing is evidence of either
an unref of freed FileData or memory corruption. Currently I'm trying out
a simplified ViewFile tree lister in an attempt at tracking down further
file_data_unref breakage.

------------------------------------------------------------------------------
WhatsUp Gold - Download Free Network Management Software
The most intuitive, comprehensive, and cost-effective network 
management toolset available today.  Delivers lowest initial 
acquisition cost and overall TCO of any competing solution.
http://p.sf.net/sfu/whatsupgold-sd
_______________________________________________
Geeqie-devel mailing list
Geeqie-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geeqie-devel

Reply via email to