setModelData crashes program



  • Hi,
    A line in my setModelData crashes the program:
    setModelData code:

    void WhatFixViewDelegate::setModelData(QWidget* editor, QAbstractItemModel* fixModel, const QModelIndex& index) const {
      QComboBox* whatComboEditor = qobject_cast<QComboBox*>(editor);
    
      QString newWhatText;
    
      **newWhatText = whatComboEditor->currentText ();//crashes**
      qDebug() << "newWhatText: " <<  whatComboEditor->currentText ();
    
      fixModel->setData (index, newWhatText, Qt::EditRole);
    
      fixModel->submit ();
    }
    
    

    The marked line causes the following error:
    0_1506523373865_Capture.JPG
    What is wrong with it?
    Thank you.



  • This post is deleted!


  • @gabor53 said in setModelData crashes program:

    Which line crashes?

    This one:

    newWhatText = whatComboEditor->currentText ()

    Then whatComboEditor may be null.

    -Michael.



  • 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 ?



  • @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??



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



  • can u paste your createEditor(..). ?



  • @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?


  • Qt Champions 2016

    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;
    }



  • @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.


  • Qt Champions 2016

    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"
    }
    

  • Lifetime Qt Champion

    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.



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



  • 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:
    0_1506565863276_Capture.JPG
    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?



  • @gabor53 said in setModelData crashes program:

    What can I add to prevent the data from disappearing?

    Sounds like a model bug. What is fixModel?



  • @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....?



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



  • @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.


  • Lifetime Qt Champion

    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.



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



  • @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?



  • 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



  • @VRonin
    That wasn't quite the point, though. I was talking about writing the single-line statement (if Qt won't offer a qboject_assert_cast<>):

    Q_ASSERT(whatComboEditor = qobject_cast<>)

    versus

    Q_ASSUME(whatComboEditor = qobject_cast<>)

    The point being that it does the assignment. So we do need the expression to evaluate, just we don't need/care to do the "assertion" part of testing the result in release build. This would not work for Q_ASSERT as (I believe) it's a macro which produces nothing (no code at all) in release build. That was when you suggested Q_ASSUME.



    • Q_ASSERT(statement);
      • Debug: if(!statement) triggerAnAssertion();
      • Release: ;
    • Q_ASSUME(statement);
      • Debug: if(!statement) triggerAnAssertion();
      • Release: statement;

    Is the difference clearer now?



  • @VRonin
    I added

     Q_ASSERT(whatComboEditor);
    

    but it behaves the same.



  • @VRonin said in setModelData crashes program:

    Sounds like a model bug. What is fixModel?



  • @VRonin

    QSqlTableModel* fixModel = new QSqlTableModel(this);
      fixModel->setTable ("Items");
      fixModel->setEditStrategy (QSqlTableModel::OnRowChange);
      fixModel->setSort (2, Qt::DescendingOrder);
    
      fixModel->select ();
    


  • @VRonin

    Is the difference clearer now?

    Yep! :) Tx.



  • @VRonin
    Any way to go around the problem?
    Would it be possible to just redraw the row in which the field was changed?


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.