I wonder if it would be prudent in the gi-test.py file to have a global
variable and inside the notify callback, re-bind it with the `scn`
parameter's value to ensure the GBoxed hack isn't going to cause
ownership bugs. Something like:
notification = None
def on_notify(sci, id, scn):
global notification
notification = scn
... rest of callback handler ...
... at end of program ...
del sci
print(notification.nmhdr.code)
I'm not very familiar with PyGI internals but my concern is that if a
reference escapes the callback handler, and PyGI calls g_boxed_copy()
under the covers but doesn't actually get a copy, that when the
reference is used later it will be pointing at garbage stack memory.
On the further thought, what if the dummy g_boxed_copy() implementation
did this instead:
static void *copy_(void*) { return 0; }
At least that way it would instantly crash any programs using it as a
normal GBoxed, and be easier for them to debug than stack corruption.
Just a though, I haven't tested it.
On 2016-04-27 05:42 AM, kugel wrote:
>
>>
If it's a non-const pointer supporting GBoxed semantics, I'd expect to
be able to be able to call g_boxed_copy() even from C and have my copy
outlive the callback. It's even more plausible for languages like Python
though, because you could close over the notification with another
function or save it into an instance variable, and it could escape that
way, just using normal everyday Python code.
> I fear (haven't check to be sure) that pygobject would just call _copy()
> for its own reasons without the programmer doing anything special. Then
> things would probably break with returning NULL.
>
It would be a good way to check if such a bug is being introduced. Even
better might be to (also) put an `assert(false);` in the copy_()
function to ensure it's not called. If it is called, then there's a bug.
> If we want to be 100% safe we would need to actually dynamically allocate
> SCNotification and create real copy/free functions. I don't think that's
> justified here though.
>
This would make it a normal GBoxed and as such wouldn't violate the
semantics of GBoxed just to get a GType. The only problem is you'd need
to copy/ref any internal pointers like the handle as well.
kugel:
It seems rather strange that a C-oriented tool requires typedefs and can’t handle explicit structs. Is there documentation or a bug report for this?
Being able to perform shallow copies of SCNotiification objects could be useful but a deep copy that includes the text field may be more complex. The valid lifetime of the text field is limited so there’d have to be a secondary allocation and code to free this. Heap allocation of original SCNotification objects by Scintilla needlessly increases costs.
The original rationale for the .gir.good file was for checking for correctness by checking for equality. By including every constant in the .gir file, this will break every time a new API or enumerated parameter is changed so will churn far too much. Maybe remove all constants from the gir.good and compare against the .gir with its constants removed. Since its XML, it may be possible to use XSLT to perform this transform.
The patch is 94K. Please try to minimise changes and isolate them to the gtk directory as much as possible. Every change made to shared headers has the potential to break other platforms or make things inconvenient (t). To me GObject introspection is an add-on, similar to a language binding or encapsulation in an object model like COM, that should not need any changes to platform-independent files. I’d really prefer not to add typedefs in Scintilla.h but will OK that if it is the only change visible on other platforms and really is required for g-ir-scanner.
The next release should be on May 23 and I’ll be on a lengthy vacation a few days later so I do not want to include anything destabilising.
Constant definitions should remain in Scintilla.h as other workflows may depend on its current format. If you need extra headers for the benefit of g-ir-scanner then create them as temporaries similar to the way that the python constants file and the autogenerated sections ScintillaEdit.h/.cpp are created by qt/ScintillaEdit/WidgetGen.py for those using Qt or PySide.
kugel:
> Here's the patch file. It mostly consists of the new contents for Scintilla-0.1.gir.good.
Shiboken, the binding generator for PySide hangs with the changes to Scintilla.h. Its possible that removing or modifying some of the changes or moving them from Scintilla.h to ScintillaWidget.h will make both targets work.
To run Shiboken, go to scintilla/qt/ScintillaEditPy and run “python sepbuild.py”. On Ubuntu this requires some packages installed and these should be enough:
apt-get install qtcreator python-dev libshiboken-dev shiboken libpyside-dev python-pyside
The filter on Scintilla.h used to strip out the APIs allows through the deprecated APIs like ’SCI_SETUSERPALETTE’. You really don’t want these.
I've found you merged my patch. Thank you a lot!
kugel:
> Alright, now I saw you reverted / backed out my change after merging, so that Colomban's patch applies cleanly. Here's my updated patch. The only change to my last patch is that the marshall functions are now auto-generated.
OK, committed.
https://sourceforge.net/p/scintilla/code/ci/64d36f2eac4de4fa1a4ef478fe784bd9b522fa3e/