setModelData crashes program

m.sue

@gabor53 said in setModelData crashes program:

Which line crashes?

This one:

newWhatText = whatComboEditor->currentText ()

Then whatComboEditor may be null.

-Michael.

dheerendra

Your assumption inside the delegate is that editor is ComboBox. First you need confirm that you are setting delegate and createEditor is returning the combobox only. Looks like in some cases createEditor of your delegate may not be returning the combobox. Can you check createEditor function ?

JonB

@gabor53 said in setModelData crashes program:

  QString newWhatText;

  **newWhatText = whatComboEditor->currentText ();//crashes**

I don't program Qt in C++. But is that ** really correct? That's a double indirection, off an initialized empty QString, if that's invalid you're writing to a random area of memory, hence a SIGSEGV? Is what you posted your actual compilable source code??

gabor53

How to check the content of whatComboEditor?
** just to mark the line that crashes

dheerendra

can u paste your createEditor(..). ?

JonB

@gabor53 said in setModelData crashes program:

How to check the content of whatComboEditor?
** just to mark the line that crashes

OMG, you put a literal ** in a legal C++ code place on the faulty line, not in a comment, where I was supposed to understand it was a comment....... That did not occur to me.

So as the others have said, where you go qobject_cast<QComboBox*>(editor) your editor parameter is either NULL, does not point to a QComboBox, or points to what was a QComboBox but is now invalid (e.g. disposed). Can't you just look at it in a debugger?

mrjj

Hi
Just a small question.
Is this code related to
https://forum.qt.io/topic/83393/centering-qcombobox-in-table-cell
where you also used a layout to center the combobox ?
I wonder if the
QComboBox* whatComboEditor = qobject_cast<QComboBox*>(editor);
then is correct ?
but add check to it
if (!whatComboEditor) {
qDebug() << "fail to cast whatComboEditor";
return;
}

gabor53

@mrjj
It is related to the referenced post. This is part of the same delegate. I added the suggested check and I got the "fail to cast whatComboEditor" message.
Here is the whole delegate:

#include "whatfixviewdelegate.h"

WhatFixViewDelegate::WhatFixViewDelegate(QObject* parent) : QStyledItemDelegate(parent) {

}

QWidget* WhatFixViewDelegate::createEditor(QWidget* parent, const QStyleOptionViewItem& option, const QModelIndex& index) const {

  Q_UNUSED(option);
  Q_UNUSED(index);

  QWidget* w = new QWidget( parent );

  QComboBox* whatComboEditor = new QComboBox(w);
  whatComboEditor->setFrame(true);
  whatComboEditor->setStyleSheet("background-color:rgb(255,217,229);" );
  whatComboEditor->setEditable(true);

  whatComboEditor->setMaximumWidth (100);
  whatComboEditor->setMinimumWidth (100);
  whatComboEditor->setMaximumHeight (30);
  whatComboEditor->setMinimumHeight (30);

  whatComboEditor->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);

  QVBoxLayout* whatLayout = new QVBoxLayout;
  whatLayout->addWidget (whatComboEditor, 0, Qt::AlignCenter);
  whatLayout->setMargin (2);

  w->setLayout (whatLayout);

  QSqlQuery queryLoadWhat("SELECT What FROM  What_Table ORDER BY What asc", db);

  while(queryLoadWhat.next()) {
    QString whatItem = queryLoadWhat.value(0).toString();
    whatComboEditor->addItem(whatItem);
  }
  return w;
}

void WhatFixViewDelegate::setEditorData(QWidget* editor, const QModelIndex& index) const {

  QComboBox* whatComboEditor = qobject_cast<QComboBox*>(editor);
  if(whatComboEditor) {
    whatComboEditor->addItem(index.model()->data(index, Qt::EditRole).toString());
    whatComboEditor->setCurrentText(index.model()->data(index, Qt::EditRole).toString());
  }
}

void WhatFixViewDelegate::updateEditorGeometry(QWidget* editor, const QStyleOptionViewItem& option, const QModelIndex& index) const {
  Q_UNUSED(index);

  editor->setGeometry(option.rect);
}

void WhatFixViewDelegate::setModelData(QWidget* editor, QAbstractItemModel* fixModel, const QModelIndex& index) const {
  QComboBox* whatComboEditor = qobject_cast<QComboBox*>(editor);
  if (!whatComboEditor) {
    qDebug() << "fail to cast whatComboEditor";
    return;
  }



  QString newWhatText;

  newWhatText = whatComboEditor->currentText ();//crashes
  qDebug() << "newWhatText: " <<  whatComboEditor->currentText ();

  fixModel->setData (index, newWhatText, Qt::EditRole);

  fixModel->submit ();
}

What should be the next step? Thank you.

mrjj

Hi
You should find the combobox in the widget with
QComboBox* whatComboEditor = editor->findChild< QComboBox*>();
if (whatComboEditor ) { ...use it... }
in the other functions since you now have a real widget in between.
as you say

QWidget* WhatFixViewDelegate::createEditor(QWidget* parent, const QStyleOptionViewItem& option, const QModelIndex& index) const {
  QWidget* w = new QWidget( parent );
....
  return w; <<< that is the place holder widget you return as "editor"
}

SGaist

In your case, you should call findChild on editor. Your QComboBox is in a layout inside a QWidget so you can't cast it like that directly.

An alternative would be to create a custom widget which wraps the QComboBox and that you could use in the same manner.

JonB

And always check the return result of a qobject_cast<>, for safety, certainly while developing! :)

gabor53

Thank you. I redid it as recommended and now it looks like this:

#include "whatfixviewdelegate.h"

WhatFixViewDelegate::WhatFixViewDelegate(QObject* parent) : QStyledItemDelegate(parent) {

}

QWidget* WhatFixViewDelegate::createEditor(QWidget* parent, const QStyleOptionViewItem& option, const QModelIndex& index) const {

  Q_UNUSED(option);
  Q_UNUSED(index);

  QWidget* w = new QWidget( parent );

  QComboBox* whatComboEditor = new QComboBox(w);
  whatComboEditor->setFrame(true);
  whatComboEditor->setStyleSheet("background-color:rgb(255,217,229);" );
  whatComboEditor->setEditable(true);

  whatComboEditor->setMaximumWidth (100);
  whatComboEditor->setMinimumWidth (100);
  whatComboEditor->setMaximumHeight (30);
  whatComboEditor->setMinimumHeight (30);

  whatComboEditor->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);

  QVBoxLayout* whatLayout = new QVBoxLayout;
  whatLayout->addWidget (whatComboEditor, 0, Qt::AlignCenter);
  whatLayout->setMargin (2);

  w->setLayout (whatLayout);

  QSqlQuery queryLoadWhat("SELECT What FROM  What_Table ORDER BY What asc", db);

  while(queryLoadWhat.next()) {
    QString whatItem = queryLoadWhat.value(0).toString();
    whatComboEditor->addItem(whatItem);
  }
  return w;
}

void WhatFixViewDelegate::setEditorData(QWidget* editor, const QModelIndex& index) const {
  QComboBox* whatComboEditor = editor->findChild<QComboBox*>();

  if(whatComboEditor) {
    whatComboEditor->addItem(index.model()->data(index, Qt::EditRole).toString());
    whatComboEditor->setCurrentText(index.model()->data(index, Qt::EditRole).toString());
  }
}

void WhatFixViewDelegate::updateEditorGeometry(QWidget* editor, const QStyleOptionViewItem& option, const QModelIndex& index) const {
  Q_UNUSED(index);

  editor->setGeometry(option.rect);
}

void WhatFixViewDelegate::setModelData(QWidget* editor, QAbstractItemModel* fixModel, const QModelIndex& index) const {

  QComboBox* whatComboEditor = editor->findChild<QComboBox*>();
  if (whatComboEditor) {
      QString newWhatText;
    
      newWhatText = whatComboEditor->currentText ();//crashes
      qDebug() << "newWhatText: " <<  whatComboEditor->currentText ();
    
      fixModel->setData (index, newWhatText, Qt::EditRole);
    
      fixModel->submit ();
  }
}

Now it doesn't crash, but when I add an item from the QComboBox, all the data from the line disappears like this:

When I reload the table all the data are back and the newly chosen item is also there (it was saved in the db). What can I add to prevent the data from disappearing?

VRonin

@gabor53 said in setModelData crashes program:

What can I add to prevent the data from disappearing?

Sounds like a model bug. What is fixModel?

JonB

@JNBarchan said in setModelData crashes program:

And always check the return result of a qobject_cast<>, for safety, certainly while developing! :)

In my nice (non-Qt) C# background, this kind of casting is built into the language. For qobject_cast<> we write:

ComboBox cb = editor as CombBox;

where the as operator returns null if editor cannot be cast to ComboBox at runtime. We use this if editor might not be a ComboBox.

However, if editor must be/is intended to be a ComboBox, we write:

ComboBox cb = (ComboBox) editor;

where the direct cast throws a runtime Exception if editor is not a ComboBox (i.e. as would return null). This saves a lot of code/accidents.

It would be nice if Qt offered a "variant" of qobject_cast<> in C++ which checked and automatically threw an Exception if the return result is NULL/0....?

VRonin

you just need to add Q_ASSERT(whatComboEditor); to have that

JonB

@VRonin said in setModelData crashes program:

you just need to add Q_ASSERT(whatComboEditor); to have that

Ah, OK. It's just that you have to remember to write that every time after each qobject_cast<>, it would be nice to have them combined in one statement. I suppose you would write:

Q_ASSERT(whatComboEditor = qobject_cast<>)

but is Q_ASSERT code removed in non-debug code and/or does it evaluate its argument more than once (assuming it's a macro)?

I would like a qobject_assert_cast<> :)

EDIT: Yes, I see you cannot use Q_ASSERT(whatComboEditor = qobject_cast<>) in non-debug as the code gets removed.

SGaist

That's not specific to Q_ASSERT, it's the same for the standard C++ assert. Always test the result of a function and not the function itself.

VRonin

If you want to use it in code that should still be present in release mode you can use Q_ASSUME

JonB

@VRonin said in setModelData crashes program:

If you want to use it in code that should still be present in release mode you can use Q_ASSUME

Um, ~~I don't think so~~ are you sure: http://doc.qt.io/qt-5/qtglobal.html#Q_ASSUME states it's a compile-time hint, and you get "undefined behaviour" if the condition is false. We need a runtime check of the return result of qobject_cast<>.

EDIT: Unless the docs are misleading, and the true implementation is via https://git.merproject.org/faenil/qtbase/commit/3b0ed624351441a2d7be45cf9582fd36955ae860 , which uses Q_ASSERT_X?

VRonin

Q_ASSUME in debug mode is like Q_ASSERT in release mode, if whatComboEditor is null the program would crash anyway so you can't do much worse.

The undefined behaviour comes from when you use the return value: if(Q_ASSUME(false)) is undefined behaviour. if it just wraps an operation you do not check anyway, Q_ASSUME does nothing to your compiled binary in release mode