GNOME Bugzilla – Bug 125142
Make libgimp 64 bit clean
Last modified: 2004-12-22 21:47:04 UTC
Apparrently theer are some parts of libgimp which assume that a pointer is the same size as an int. libgimp should be 64 bit clean before the pre-releases. It would be nice if someone who knows more about this than me could outline what needs to be done for this, and where exactly the code isn't 64 bit clean. Cheers, Dave.
From discussion on IRC, this has to do with GINT_TO_POINTER macros using, specifically, libgimpwidget and gimp_option_menu_new2(). Essentially, there is a common case where the callback data is an enum, and rather than allocating a block of memory, the int is stored as the pointer, and to avoid warnings (which are safe to ignore on gcc and probably other compilers, if your callback is correct) you need to add the GINT_TO_POINTER and POINTER_TO_GINT type-casting macros. Getting rid of this case entirely would require one of two approaches: 1. a different gimp_option_menu_new2() which does the casting and calls the original. However the callback signature also need to be changed to get rid of the need for POINTER_TO_GINT. 2. actually allocating memory for the enums. Yosh likes the idea of using registered enums, so that PDB entries could be nicer. In either case, every call back that does this would need to change. Using some grep magic, the number of files that use this trick is 93, though doubtless some are not relevent to this particular case. Use this to get a list of the files: find . -name '*.c' -or -name '*.h' | xargs grep -l -E '{GINT_TO_POINTER)|(POINTER_TO_GINT)'
If I understand this correctly, it's a complete overhaul of the libgimp API that's being proposed. If that's the case (please comment, someone who can confirm), I vote bumping this to be done for 3.0 - we're far too close to a 2.0 release to consider that kind of drastic change. 2.2 should happen in the middle of next year, so we would be delaying the inclusion of this for 6 or 8 noths, which would give people time to put together a proposal, have a number of people work on this and so on. If we try to force this into 2.0, we'll end up delaying things at least a few weeks, and more likely a month or 2. Voting to bump this to 3.0 (future). Dave.
There is o complete overhaul needed. gimp_option_menu_new2() just needs to be redone or (preferably) wrapped by a much easier API which deals with integer values. I see no other 64bit issue and even this one is not a real issue because we have GINT_TO_POINTER() (which is not broken, it's only ugly). We should not bump it to 3.0.
The (old) gimp_dialog_new() API is part of the problem; adding a dependency.
Is there any objections to making gimp_option_menu_new and gimp_radio_group_new be versions of their respective new2s that take ints and rename the current ones to _old? Looking at the tree pretty much every usage case is mapping an integer value to a variable.. so that seems the common thing. Could have gimpcompat.h do a #define to get the old API back. How does this sound to people?
So you are basically suggesting to replace gpointer item_data with gint item_id. Did I understand that correctly? Do we need the old API at all?
Yes, you understood correctly. I don't think anything in our tree needs the old api, but I don't know about out of tree plugins. The old api is potentially useful, but not really the common case.
Sounds good to me. Dave.
2003-11-14 Manish Singh <yosh@gimp.org> * libgimpwidgets/gimpwidgets.[ch]: add gimp_int_option_menu_set_history as a wrapper for gimp_option_menu_set_history. 2003-11-14 Manish Singh <yosh@gimp.org> * libgimpwidgets/gimpwidgets.[ch]: implemented gimp_int_option_menu_new and gimp_int_radio_group_new, which are the same as gimp_option_menu_new2 and gimp_radio_group_new2, but they take integers as values to map instead of gpointers, which avoids casts in pretty much all uses of it in the tree.