Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault when adding a layer #36910

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

pblottiere
Copy link
Member

@pblottiere pblottiere commented Jun 2, 2020

Fixes a segmentation fault happening when I try to add a vector layer on Archlinux.

The string returned by sqlite3_mprintf has to be freed.

@github-actions github-actions bot added this to the 3.14.0 milestone Jun 2, 2020
@pblottiere
Copy link
Member Author

@pblottiere pblottiere requested a review from elpaso June 2, 2020 14:37
@pblottiere pblottiere added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Jun 2, 2020
@rduivenvoorde
Copy link
Contributor

Will build and test it

@rduivenvoorde
Copy link
Contributor

Seems to work with me, no more segfault on opening vector data dialog.
I did have to clean first though.

@pblottiere
Copy link
Member Author

Seems to work with me, no more segfault on opening vector data dialog.

Good. Thanks for testing @rduivenvoorde 👍

@nyalldawson nyalldawson merged commit 07eca3e into qgis:master Jun 2, 2020
@3nids
Copy link
Member

3nids commented Jun 5, 2020

@pblottiere as mentionned in the commit, I am getting a crash on mac os.
Reverting this fixes the crash...
see 07eca3e#r39692642

@pblottiere
Copy link
Member Author

pblottiere commented Jun 5, 2020

Hi @3nids,

Mmmm... So if we use sqlite3_mprintf, we have a crash on Debian (and probably other Debian based distrib like Ubuntu) and at least Archlinux. But if we use QgsSqlite3Mprintf, we have a crash on mac os.

But QgsSqlite3Mprintf is used everywhere in QGIS source code instead of sqlite3_mprintf, so I don't think there's an issue with QgsSqlite3Mprintf itself...

A stupid idea: can you try to use a fresh QString for the second call to QgsSqlite3Mprintf instead of reusing sql?

@3nids
Copy link
Member

3nids commented Jun 5, 2020

A stupid idea: can you try to use a fresh QString for the second call to QgsSqlite3Mprintf instead of reusing sql?

no it doesn't help. Also, the crash happens within the call of QgsSqlite3Mprintf.

Shall we do an #ifdef on the OS?

@3nids 3nids mentioned this pull request Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants