[Freeciv-Dev] Re: (PR#2922) Revised version of data-reorganizattion patch

["Eric S. Raymond" <esr@xxxxxxxxxxx>]

Jason Short via RT <rt@xxxxxxxxxxxxxx>:
> Hmm, true.  Perhaps it would be even better to add the new files, then 
> provide a patch for the code and .tilespec files.

That was my original plan.

> Perhaps there is a reason.  But if so, I'd rather find it and document 
> it rather than just leave it...

The documented thing, doing a simple free(), seems to work.
 
> I'm pretty sure POSIX says something like "MAX_PATH_LEN, if defined, 
> gives the maximum length of any path string".  And HURD does not define it.
> 
> But don't worry about it; if it's broken somebody will fix it someday...

HURD.  Hah.  It is to laugh.
 
> Err, yes, but you don't want to wantonly leak memory before that 
> (although this is rare, since there are not many duplicates loaded).  If 
> the last pointer to a sprite goes away, it should be freed.  Before a 
> duplicately-referenced sprite would only be loaded once, so there could 
> be multiple references and it wasn't safe to free it when it was 
> replaced.  But if that's no longer the case then we might as well do so.

Given that the rate of duplicates is very low, I'm far from sure this
change would be worth the marginal resk of introducing a double-free bug.

Memory management sucks.  Has anyone considered re-implementing this puppy
in Python? :-)

> Speaking of which, have you tried rereading tilesets with the patch?  Do 
> this at runtime using the local option, and also try 
> disconncting/reconneecting to running servers.

I'll run some tests.  I don't expect problems; the new memory allocation 
scheme is actually simpler than the old.
 
> They need to work with "make dist" and "make distcheck", which means 
> they need to be in a Makefile.am under EXTRA_DIST.  I'm pretty sure 
> using wildcards and such here will break "make distcheck" - I once tried 
> to "simplify" things this way and eventually found out my error.

I'll experiment.  It's going to be a serious pain in my butt if I can't 
wildcard there -- that would pretty much torpedo the plan to make
dropping in nations not require patching any files.

Yes, it fails.  Now I'm going to have to come up with something
clever, dammit.
-- 
                <a href="http://www.catb.org/~esr/";>Eric S. Raymond</a>