Project

General

Profile

Bug #10199

crash because g_srsplit returns NULL

Added by Stephan Pleines over 5 years ago. Updated over 5 years ago.

Status:
Fixed
Priority:
Low
Category:
Lighttable
Start date:
11/18/2014
Due date:
% Done:

100%

Estimated time:
Affected Version:
git development version
System:
all
bitness:
64-bit
hardware architecture:
amd64/x86

Description

In _folder_tree.

In 720 the following fixes it:
while ((pch != NULL) && (pch[level] != NULL))

I'm on the master branch, I guess that means "git development version"?

If you need a backtrace I can attach one, but the problem seems fairly obvious.

Thanks,
steple

Associated revisions

Revision 751e12c2 (diff)
Added by Tobias Ellinghaus over 5 years ago

Don't crash on broken film roll db entries

This takes care of the first half (and the actual crash) of bug #10199.

History

#1 Updated by Stephan Pleines over 5 years ago

That should be g_strsplit of course.

#2 Updated by Stephan Pleines over 5 years ago

And GLib-CRITICAL **: g_strsplit: assertion 'string != NULL' failed
so NULL is probably passed to g_strsplit.

#3 Updated by Roman Lebedev over 5 years ago

  • % Done changed from 0 to 20
  • Status changed from New to Incomplete

If you are reporting a crash, you definitely need to provide a backtrace.

#4 Updated by Tobias Ellinghaus over 5 years ago

Looking at the code it seems that you are running into a weird situation with a filmroll having NULL as its folder. Could you please see what select * from film_rolls where folder is NULL; returns when run inside sqlite3 ~/.config/darktable/library.db?

#5 Updated by Stephan Pleines over 5 years ago

Yes, I created the situation by trying to move files from one folder to another. That failed with a message like "failed to create film roll". I guess that created a situation in which the sql db returns, which isn't handled subsequently.

I recreated my DB and was able to reproduce it.

#6 Updated by Tobias Ellinghaus over 5 years ago

  • System changed from Ubuntu to all
  • % Done changed from 20 to 50
  • Target version set to Candidate for next minor release
  • Assignee set to Tobias Ellinghaus
  • Status changed from Incomplete to In Progress
  • Category set to Lighttable
So two things should be done:
  • safeguard against empty folder strings (a little different than what you proposed)
  • gracefully handle errors to not leave broken database entries

#7 Updated by Stephan Pleines over 5 years ago

Well personally I'm a big fan of never assuming that I get a valid pointer.

Anyway, this seems to be resolved insofar as that I am unable to reproduce it because I can no longer break the database (moving to a different folder now works again).

#8 Updated by Tobias Ellinghaus over 5 years ago

  • % Done changed from 50 to 100
  • Status changed from In Progress to Fixed

If you look at 751e12c2 you will see that I did guard against NULL in that part of the code. Just outside of the loop so we avoid comparing the same thing over and over again.

#9 Updated by Stephan Pleines over 5 years ago

Then I'm a fan!

What if g_strsplit returns NULL?

#10 Updated by Tobias Ellinghaus over 5 years ago

That can only happen for an empty string as input which would indicate another bug somewhere else.

Also available in: Atom PDF

Go to top