Hi,

Latest marble master on ubuntu crashes at startup with :

Code: Select all

Determining remote bookmark state.
Style reset requested.
Style reset requested.
Style reset requested.
Style reset requested.
Style reset requested.
Style reset requested.
Remote bookmark timestamp is  ""
Merging remote and local bookmark file
bool Marble::BookmarkManager::loadFile(const QString&) Loading Bookmark File: "/home/filnet/.local/share/marble/bookmarks/bookmarks.kml"
Could not open/parse file "/home/filnet/.local/share/marble/bookmarks/bookmarks.kml"
Could not parse file "/home/filnet/.local/share/marble/bookmarks/bookmarks.kml"
This could be caused by a previous broken bookmark file. Trying to recover.
Null pointer in call to GeoDataTreeModel::addFeature (parent  0x83af81c  - feature 0x0 )
Segmentation fault (core dumped)


Happens also after deleting /home/filnet/.local/share/marble directory.

Philippe.

Are you able to install debugging symbols, and generate a backtrace using the KDE Crash Handler, or alternately - using gdb manually (if you're familiar with it - I assume you are considering core files have been activated).

Also, which distribution are you on, and which version of Marble/KDE are you using?

I am on Ubuntu 12.04 running in Virtual Box...

Marble is the from latest master (i.e. I pulled the latest changes from yesterday).
And I am doing a qt only build against qt5.1.1.

I'll try to salvage a valid bookmark file from my Windows install of Marble and see if Marble runs fine with it.
If not I'll try to generate a backtrace of the crash.

Dropping a valid bookmark.kml file in ~/.local/share/marble/bookmarks fixed the issue.
Marble then ran fine.

Removing that file led to the same crash.

Is that enough information or do you still need the backtrace?

It seems Marble fails to recover from a incorrect bookmark.kml file - so I suspect a backtrace would still be useful. I'll leave it up to the developers to comment on that however - particularly considering this is non-released code.

A quick look at method BookmarkManager::loadFile() shows that it is probably broken with regard to handling an absent or corrupted bookmark file.

If you're able to provide a patch, I would suggest uploading it to KDE Reviewboard for the Marble developers to review.
Reviewboard can be found at https://git.reviewboard.kde.org/

This is the problematic code :

If the openFile call fails and returns a null document then the rest of the code will misbehave.
You can that the code contains some Todo...

If I am given some directions (especially with regard to the intent behind "recover") then I can try to submiit a fix.

Code: Select all

bool BookmarkManager::loadFile( const QString &relativeFilePath )
{
    d->m_bookmarkFileRelativePath = relativeFilePath;
    QString absoluteFilePath = bookmarkFile();

    mDebug() << Q_FUNC_INFO << "Loading Bookmark File:" << absoluteFilePath;

    if (absoluteFilePath.isEmpty())
        return false;

    if ( relativeFilePath.isNull() )
        return false;

    GeoDataDocument *document = openFile( absoluteFilePath );
    bool recover = false;
    if ( !document ) {
        mDebug() << "Could not parse file" << absoluteFilePath;
        mDebug() << "This could be caused by a previous broken bookmark file. Trying to recover.";
        /** @todo: Remove this workaround and return false around Marble 1.4 */
        recover = true;
        // return false;
    }

    d->m_treeModel->removeDocument( d->m_bookmarkDocument );
    delete d->m_bookmarkDocument;
    d->m_bookmarkDocument = document;

    d->m_treeModel->addDocument( d->m_bookmarkDocument );

    if ( recover ) {
        updateBookmarkFile();
    }

    emit bookmarksChanged();
    return true;
}


EDIT : I am really new to Marble but willing to learn and contribute so any help is welcome...

Seems to be a regression introduced in http://quickgit.kde.org/?p=marble.git&a ... a2b73cba4f
Can you change the current code

Code: Select all

    d->m_treeModel->addDocument( d->m_bookmarkDocument );

    if ( recover ) {
        updateBookmarkFile();
    }


to

Code: Select all

    if ( recover ) {
        d->resetBookmarkDocument();
        updateBookmarkFile();
    } else {
        d->m_treeModel->addDocument( d->m_bookmarkDocument );
    }


and see if that fixes it?

Hi Earthwings,

The proposed code changes fixes the issue.
I tested with missing and corrupted bookmark file.

How do I submit a patch. I'd like to do it as a way to learn the process of submitting a patch.

The best way to provide patches is through https://git.reviewboard.kde.org/ like bcooksley said.
You can create the needed diffs directly with git, e.g. by calling git diff. For Windows, msys git allows doing that.

Given that today the second beta is tagged I already pushed this patch now in order to have it become part of Beta 2. See also https://bugs.kde.org/show_bug.cgi?id=327444