On 26/10/2013 00:22, Lex Trotman wrote:
On 26 October 2013 00:01, Nick Treleaven <nick.trelea...@btinternet.com>wrote:
On 24/10/2013 10:24, Lex Trotman wrote:
Okay, but you still agree that doc->is_valid should be removed eventually?
That's a step forward:)
Of course I agree. So its not a terribly big step:)
I only skimmed the discussion, but how can we remove that?
Given your experience it would be good if you could read the discussion and
give your perspective on what the previous posters have discussed regarding
handling doc pointers properly.
I read the discussion.
In summary, I think there are 3 issues:
* forgetting to check is_valid when iterating documents
* freeing memory
* code that uses is_valid to check if a document still exists, which is
bad practice
I think freeing memory is a non-issue, I think you (Lex) agree with me
on that. It's not a memory leak, it may be reused later.
The first point needs to be handled before the last one (assuming each
are worth fixing). I think we can:
* remove GeanyDocument::index and fix any code that uses it
* review all code indexing documents_array and fix any code retaining an
index (I expect that is rarer than code using GeanyDocument::index).
* change documents_array to only contain valid documents, i.e. on close
we swap the last valid document pointer with the pointer being removed,
and decrement the length by 1.
* use a free list for invalid documents
The last point is important to ensure no memory corruption bugs are
introduced. I think it is harder to find the code that retains document
pointers (and there is some), so I recommend we don't free document
memory. At least, we would need a better reason to do so IMO than just
memory usage, which is insignificant.
Now, once we've done that it's no longer needed to check is_valid when
iterating documents. Possibly we could remove is_valid, but I think that
is less important to do. Note that if we do all the steps above, we
don't need to break the API, only the ABI. I can't guarantee that bugs
won't be introduced due to these changes, but it seems fairly reliable.
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
