Created attachment 113575 [details] [review]
Fix for the pixel region issues and some color-related ones

Patch makes sense to me, applied (after removing tab chars) to trunk, rev 26011:

2008-06-28  Martin Nordholts  <martinn@svn.gnome.org>

	* pygimp-pdb.c:
	* pygimp-tile.c:
	* pygimp-colors.c: Applied patch from Ed Swartz that makes use of
	Py_ssize_t:s instead of int:s to better utilize 64-bit
	systems. (Bug #540629.)

Reverted :( Reopening.

2008-06-28  Michael Natterer  <mitch@gimp.org>

	* pygimp-pdb.c:
	* pygimp-tile.c:
	* pygimp-colors.c: revert last patch because Py_ssize_t is
	undefined in some (older?) pythons. Needs more investigation.

Oops, didn't think of that. This page recomends how to deal with it:
http://www.python.org/dev/peps/pep-0353/

Ed, could you perhaps write a new patch? (Don't use tab characters either please.)

Ok... sorry I didn't notice this was a Python 2.5 feature.  Thanks for the PEP reference!

Well, there are a ton of warnings related to the int -> Py_ssize_t change in this module, but I had only addressed the ones I originally noticed.  I'll provide a patch that cleans up all these Python-related warnings.  

Created attachment 113621 [details] [review]
patch respecting differences in Python 2.4 vs 2.5

I fully expect someone to massage my questionable placement of the version-checking code into pygimp-api.h.  I didn't want to add a new #include or a set of autoconf tests for this... I'm really lazy!

Also, I tried not to add any new tabs in the patch, but that doesn't make all the existing ones in this plug-in go away  :)

That was a bit more complicated, I suggest we wait with applying this upstreams to not complicate the merge of the Python scripting enhancements, which should happen for 2.8.

With all due respect, this is a crasher, and it's going to wait two releases?

It's not a complicated bug at all.  It's writing 64-bit values into adjacent 32-bit variables on the stack.  They overwrite each other and either make pixel region accesses fail with bogus assertion errors or crash pygimp.

I'm sorry if my trying to clean up related warnings made this bug look complicated or unimportant, but it makes pygimp absolutely useless for bit blts. 

Simple test:

============

#!/usr/bin/env python

from gimpfu import *
from types import *

def bug_540629(image, drawable):
    frgn = drawable.get_pixel_rgn(0, 0, drawable.width, drawable.height, False, False)
    imagestring = frgn[:,:].tostring()
    trgn = drawable.get_pixel_rgn(0, 0, drawable.width, drawable.height, True, False)
    trgn[:,:] = imagestring  # this should fail with assertions or crash without the fix
    return

# Register with The Gimp
if __name__ == "__main__":
                
    register(
        "plug_in_bug_test",
        "Test bug 540629",
        "64-bit writes to 32-bit ints",
        "EJS",
        "EJS",
        "2008-06-29",
        "<Image>/Python-Fu/Bug 540629",
        "RGB*, GRAY*",
             [
              ],#controls
             [],
                bug_540629)  # how

    main()

=======

Make a new image and invoke Python-Fu > Bug 540629 and you will see the crash in pygimp.  This fails with any slicing (rect, row, or column accesses).  Only single pixel access works. 

(simple.py:30738): LibGimp-CRITICAL **: gimp_pixel_rgn_get_rect: assertion `buf != NULL' failed
/home/ejs/.gimp-2.4/plug-ins/simple.py: fatal error: Segmentation fault

By all means revisit the warnings addressed by my last patch later if this code is going to change a lot.  But it's going to be a headache to rebuild pygimp by hand for the next several years just to work around this simple problem.  (And I'm not going back to 32-bit land ;)

Ok let's aim for 2.6 then.

Created attachment 113718 [details] [review]
Resolves problems with 64bit machines and python 2.5

The patch should fix the problem. It only replaces int with Py_ssize_t where needed.(E.g when PySplice_GetIndices is used). Eds patch replaced int in some places where not needed, like pr_resize (Which maybe even introduced some other crashers).
Although I did not have the possibility to test it on a 64bit machine. Ed can you do this?

Imo it should be applied asap because it is a major problem that core functionality of pygimp is unusable on 64bit machines.

And please note that it is still possible to crash pygimp by accessing a pixelregion with invalid indices. Also on 32bit machines.

Please commit to the stable gimp-2-4 branch and to trunk.

Fixed in SVN

2008-07-03  Lars-Peter Clausen  <lars@metafoo.de>

	* plug-ins/pygimp/pygimp-drawable.c
	* plug-ins/pygimp/pygimp-vectors.c
	* plug-ins/pygimp/pygimp-display.c
	* plug-ins/pygimp/pygimp-image.c: Added checks to ensure that a python
	object only is created if its id is valid. Fixes bug #536403.
	* plug-ins/pygimp/pygimp-pdb.c
	* plug-ins/pygimp/pygimp-tile.c
	* plug-ins/pygimp/pygimp-colors.c
	* plug-ins/pygimp/pygimp.h: Fix crashing when pygimp is used with
	python-2.5 on 64 bit systems. Fixes bug #540629.

*** Bug 551849 has been marked as a duplicate of this bug. ***