[PyKDE] Implementation of KConfigSkeleton::addItemUInt

[Jim Bublitz]

On Wednesday 01 September 2004 00:26, Michael Franz Aigner wrote:
> Subject: Implementation of KConfigSkeleton::addItemUInt
>
> Hello,
>
> While using KConfigSkeleton in an application of mine, I noticed that
> KConfigSkeleton::addItemInt/UInt are not working. The value of the
> returned configuration item would change randomly while the
> application runs.
>
> Tracking down the problem i found out that the C++ code generated by
> sip passes a reference to temporary integer variable to the C++
> function, whereas a reference to a non-temporary variable would be
> needed. This is because KConfigSkeleton remembers the reference and
> uses it later in the application. This reference ends up pointing to
> the location on the stack where this temporary variable once was but
> not anymore is. As this location on the stack is reused, the variable
> the reference is pointing to seemingly changes its value randomly as
> the application runs.
>
> In search for a way to make KConfigSkeleton::addItemUInt work, i came
> up with the following. The patch is against PyKDE 3.11.3:
> http://amfranz.com/pykde/PyKDE-3.11.3-intref.diff
>
> Usage would be something like:
>
> ===
> from kdecore import KConfigSkeleton, UIntRef
>
> class Preferences(KConfigSkeleton):
>     def __init__(self, *args):
>         KConfigSkeleton.__init__(self, *args)
>
>         self.some_int_setting = kdecore.UIntRef()
>         self.some_int_setting_property =
> self.addItemUInt('some_int_setting', self.some_int_setting, 1000)
>
>         print "value is: %d" % self.some_int_setting.getValue()
> ===
>
> Could you comment on that?
> Maybe there a simpler way to make KConfigSkeleton::addItemInt work,
> w/o introducing a new class, like 'UIntRef' in the above example?

This is what KDE does:

KConfigSkeleton::ItemBool *KConfigSkeleton::addItemBool (onst QString &name,
      bool &reference,  bool defaultValue, const QString &key )
{
  KConfigSkeleton::ItemBool *item;
  item = 
   new KConfigSkeleton::ItemBool( mCurrentGroup, key.isNull() ? name : key,
                                                     reference,defaultValue );
  addItem( item, name );
  return item;
}

In Python you should be able to do:

   self.reference = True
   self.item = KConfigSkeleton.ItemBool (mCurrentGroup, name, reference,          
                          defaultValue)
   self.addItem (self.item, name)

I'm not sure I handled the key/name distinction correctly, but it should be 
doable. I'm also not sure if that fixes the "scalar-by-reference" problem 
either, but I think it does.

In that case, you should be able to overload addItemBool in Python to take 
self.item instead of the bool reference. Seems to me that should work, 
although I obviously don't have the details worked out. I'd prefer that to 
adding the additional classes.  What I'd like to avoid is "special" PyKDE 
methods/classes if at all possible. In this case, the required classes 
*almost* exist already.

I could also modify the addItem* calls to create the referenced scalar on the 
heap instead of as a temporary. I'm not sure how it would get deleted though 
- probably in the dtors for the Item* objects.  There are some other things I 
could try along those lines, including not calling the C++ addItem* methods 
at all, but doing the equivalent on the sip C++ code side if that proved 
useful.

Would you be able to try out the Python suggestion above?

It's going to take me some time to work through this, as I don't have any 
working code that uses KConfigSkeleton, and whatever solution I try, I'd like 
to be able to test here. I also need to test out the Item<scalar> classes and 
verify that the Python code (including "casting") works correctly - those 
might also need some work. It's likely to take some time.

The underlying problem is that all of this is built off of 
KConfigSkeletonGenericItem, which is a template class and I've taken the 
"easy" way around that. 

Very nice job on the C++ code fix though - it looks like it should work 
correctly and it doesn't seem like it would have the memory leak problem 
mentioned above.

Jim