Fix assertion failure when creating a python script in kig
Needs ReviewPublic

Authored by paolini on Dec 31 2019, 4:24 PM.

Details

Reviewers
None
Group Reviewers
KDE Edu
Summary

This is a proposal (suggested by Franco Pasquarelli) to fix bug
https://bugs.kde.org/show_bug.cgi?id=401512

Diff Detail

Repository
R331 Kig
Lint
Lint Skipped
Unit
Unit Tests Skipped
paolini created this revision.Dec 31 2019, 4:24 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptDec 31 2019, 4:24 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
paolini requested review of this revision.Dec 31 2019, 4:24 PM
paolini added a reviewer: KDE Edu.
narvaez added a subscriber: narvaez.
narvaez added inline comments.
scripting/python_scripter.cc
530

Is there any valid case in which (objectvect.begin() + i)->ptr() is NULL? According to the Python documentation we only need XINCREF if the argument may be NULL, otherwise we can use Py_INCREF.

kkofler added inline comments.Jan 1 2020, 2:50 PM
scripting/python_scripter.cc
530

I don't know. I suspect not, but on the other hand, having the null check is safer. But if you are sure the null check is redundant, then it should be safe to change this.

It is likely that objectvect.begin() + i)->ptr() is'nt NULL, so XINCREF can be used, but I wanted to be sure since we are debugging

On second thought I would vote for Py_INCREF... an object_imp can change dinamically, but I think the worst case would be an "InvalidObject", which still is an object.
We should apply this patch in the git repository; I think I can do it myself.

Pushed in git@git.kde.org:kig branch master. I should do the same in Applications/19.12, I guess

patch applied to release/19.12 also

cfeck removed a reviewer: kkofler.Jan 21 2020, 9:18 AM
cfeck edited subscribers, added: cfeck; removed: francopasquarelli, narvaez.

If this is completed, please close this and the linked bug.

Phab removing subscribers again ...

Closing... I was unsure what to do. The commit that I made contained the wrong reference, so that there is no evidence here that the patch was applied.
BTW that was

commit 3f26783588ea2510ba3d7cd2ac39292d3b1706a3
Author: Maurizio Paolini <paolini@dmf.unicatt.it>
Date: Wed Jan 8 09:44:28 2020 +0100

Fix https://bugs.kde.org/show_bug.cgi?id=401512

added Py_INCREF for each argument of the python script