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] avoid segfault in case of missing mountpoint #204

Closed
wants to merge 1 commit into from

Conversation

mizhka
Copy link

@mizhka mizhka commented Sep 26, 2022

Simple fix to avoid segfault in DirReadJob::shouldCrossIntoFilesystem in logging logic

Simple fix to avoid segfault in DirReadJob::shouldCrossIntoFilesystem in logging
logic
@shundhammer
Copy link
Owner

Thank you. I shifted the code a bit around, though.

But I am curious: Did you actually get a segfault? Of course it's always safer (and strongly advised) to check the result of any operation that can return a null pointer, and I hope that I did that everywhere else in QDirStat; but under what circumstances does it actually get to this code if that URL isn't actually a mount point? It might still legitimately happen, but when and how?

@mizhka
Copy link
Author

mizhka commented Sep 26, 2022

Yes, on FreeBSD there is no found mountpoint for /net, /dev and /proc.
Test case is to run scan for root path with cross-mount check.
Also last commit should be adjusted:

if ( mountPoint ) => if ( ! mountPoint )

Actual backtrace (before adjustment mentioned above) :

(lldb) thread backtrace
* thread #1, name = 'qdirstat', stop reason = signal SIGSEGV
  * frame #0: 0x0000000825d151ba libQt5Core.so.5`QString::indexOf(QString const&, int, Qt::CaseSensitivity) const [inlined] QString::unicode(this=0x0000000000000008) const at qstring.h:1080:41
    frame #1: 0x0000000825d151ba libQt5Core.so.5`QString::indexOf(this=0x0000000000000008, str=0x0000000820432ca8, from=0, cs=CaseSensitive) const at qstring.cpp:3764:50
    frame #2: 0x00000000002a8225 qdirstat`QString::contains(this=0x0000000000000008, s=0x0000000820432ca8, cs=CaseSensitive) const at qstring.h:1358:10
    frame #3: 0x00000000003905a5 qdirstat`QDirStat::MountPoint::isSystemMount(this=0x0000000000000000) const at MountPoints.cpp:106:20
    frame #4: 0x00000000002d1021 qdirstat`QDirStat::DirReadJob::shouldCrossIntoFilesystem(this=0x000000083afa6f80, dir=0x000000086c56c100) const at DirReadJob.cpp:182:16
    frame #5: 0x00000000002d27a3 qdirstat`QDirStat::LocalDirReadJob::processSubDir(this=0x000000083afa6f80, entryName=0x0000000820433078, subDir=0x000000086c56c100) at DirReadJob.cpp:421:40
    frame #6: 0x00000000002d1c21 qdirstat`QDirStat::LocalDirReadJob::startReading(this=0x000000083afa6f80) at DirReadJob.cpp:290:7

@shundhammer
Copy link
Owner

Yes, on FreeBSD there is no found mountpoint for /net, /dev and /proc. Test case is to run scan for root path with cross-mount check.

OK, I see. Thanks for clarifying.

Also last commit should be adjusted:

if ( mountPoint ) => if ( ! mountPoint )

Duh. You are right, of course. I was too hasty. ;-(

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants