Skip to content

Commit

Permalink
QJniArray: don't subclass QJniObject
Browse files Browse the repository at this point in the history
QJniObject is not prepared for being subclassed (no virtual destructor),
and doing so is formally UB.

Instead of making QJniArray(Base) a QJniObject subclass, give it a
QJniObject member and make it convertible to/from QJniObject. Existing
code still works with this change, even though it removes all the
inherited QJniObject APIs for accessing fields and methods. However, as
the Java array classes have a very narrow and well-defined API anyway we
could, if needed, add those as C++ member functions instead of going
through calling-by-string.

Found during API review.

Pick-to: 6.7
Task-number: QTBUG-119952
Change-Id: Ic4437116eed5e15226449bdabe48ab657cb14dc3
Reviewed-by: Marc Mutz <[email protected]>
  • Loading branch information
vohi committed Jan 29, 2024
1 parent 23fb1c5 commit 101ab27
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
23 changes: 16 additions & 7 deletions src/corelib/kernel/qjniarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ struct QJniArrayIterator
{}
};

class QJniArrayBase : public QJniObject
class QJniArrayBase
{
// for SFINAE'ing out the fromContainer named constructor
template <typename Container, typename = void> struct CanConvertHelper : std::false_type {};
Expand All @@ -99,9 +99,15 @@ class QJniArrayBase : public QJniObject
using size_type = jsize;
using difference_type = size_type;

operator QJniObject() const { return m_object; }

template <typename T = jobject>
T object() const { return m_object.object<T>(); }
bool isValid() const { return m_object.isValid(); }

size_type size() const
{
if (jarray array = object<jarray>())
if (jarray array = m_object.object<jarray>())
return jniEnv()->GetArrayLength(array);
return 0;
}
Expand Down Expand Up @@ -160,22 +166,25 @@ class QJniArrayBase : public QJniObject
~QJniArrayBase() = default;

explicit QJniArrayBase(jarray array)
: QJniObject(static_cast<jobject>(array))
: m_object(static_cast<jobject>(array))
{
static_assert(sizeof(QJniArrayBase) == sizeof(QJniObject),
"QJniArrayBase must have the same size as QJniObject!");
}
explicit QJniArrayBase(const QJniObject &object)
: QJniObject(object)
: m_object(object)
{}
explicit QJniArrayBase(QJniObject &&object) noexcept
: QJniObject(std::move(object))
: m_object(std::move(object))
{}

JNIEnv *jniEnv() const noexcept { return QJniEnvironment::getJniEnv(); }

template <typename ElementType, typename List, typename NewFn, typename SetFn>
static auto makeArray(List &&list, NewFn &&newArray, SetFn &&setRegion);
template <typename List>
static auto makeObjectArray(List &&list);

private:
QJniObject m_object;
};

template <typename T>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static QString fetchFont(const QString &query)
HashSet innerSet;
Q_ASSERT(outerList.isValid() && innerSet.isValid());

for (QJniObject signature : signatures) {
for (const auto &signature : signatures) {
const QJniArray<jbyte> byteArray = signature.callMethod<jbyte[]>("toByteArray");

// add takes an Object, not an Array
Expand Down

0 comments on commit 101ab27

Please sign in to comment.