PVS V522: PathMonitor: NULL pointer dereference

Change-Id: I77df9c18960eee0183145441d3a1bd4ceb112147
Reviewed-on: https://review.haiku-os.org/c/1662
Reviewed-by: waddlesplash <waddlesplash@gmail.com>

More class/struct mixup fixes.

Spotted by Clang.

BPathMonitor: Fix locking order reversal introduced in 8599f4b.

The sLocker was used as an outer lock with the sLooper locked within.
The sLocker therefore can't be used within MessageReceived() as that
could lead to deadlocks due to reversal of the locking order.

Instead of two locks, just use locking the looper for all serialization.
While this has a higher overhead to using a BLocker (due to the looper
list locking and lookups) this shouldn't be too problematic.

BPathMonitor: Remove unused headers, some whitespace cleanup.

BPathMonitor: Lock on incoming node monitor messages.

The lock was only acquired when paths to watch were added or removed,
protecting the data structures against concurrent modification due
to addition/removal of entries by the API user.

Locking is also required for node monitor messages since these can
trigger the data structures to be modified (due to recursive watching
and new directories becoming available or due to resyncing of modified
ancestor chains).

Previously it was possible to corrupt the data structures when node
monitor messages were received while still starting to watch a directory
structure. This was especially likely in the case of watching devfs
directories, as accessing these can trigger device scanning which in
turn could possibly add new device entries. Either the path monitor
looper or the API user would then trip over the corrupted data
structures.

Probably fixes #11280. Although I was only able to reproduce crashes
on the API side, corruption of the hash tables and corresponding endless
loops are quite plausible.

Possibly also fixes #12412 if the input_server was in the process of
starting to watch entries. It's hard to tell due to the lack of a back
trace but would fit the crashes I was able to reproduce with a synthetic
test case.

PathMonitor: check for NULL before dereferencing.

* should fix #10277.

Fix Coverity CID 1108465: Use after free

Signed-off-by: Ingo Weinhold <ingo_weinhold@gmx.de>

BPathMonitor: add "from path" to B_ENTRY_MOVED notifications

BPathMonitor: rewrite

This resolves all issues the test suite uncovered. It should also deal
with hard links correctly, though that hasn't been tested. Still
unsupported are:
* changes due to mounting/unmounting a volume,
* tracking of symlinks in the path components.

BPathMonitor: pass BMessenger by reference

B_WATCH_FOLDERS_ONLY -> B_WATCH_DIRECTORIES_ONLY

Stick to the nomenclature generally used in the public API.

BPathMonitor: PathHandler::_NotifyTarget(): simplify

* Add optional entry_ref return parameter to _HasFile().
* Simplify _NotifyTarget() by using _HasDirectory() and _HasFile().

BPathMonitor: make the node watching mechanism configurable

Add inner class BWatchingInterface and method SetWatchingInterface().
This abstracts the calls to watch_node() and stop_watching(), thus
making it possible to use the path monitor in Tracker.

BPathMonitor: use pthread_once for initialization

PathMonitor.cpp: some style cleanup

Drop into debugger if the looper can't be locked.
This gets triggered randomly by net_server.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@39934 a95241bf-73f2-0310-859d-f6bbb57e9c96

Resolved TODO about recursively removing watched files and folders when a
directory is moved outside the watched hierarchy or deleted entirely. It
involves another TODO, because the generated notifications are B_ENTRY_REMOVED,
while B_ENTRY_MOVED is probably more correct.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@37049 a95241bf-73f2-0310-859d-f6bbb57e9c96

Fixed various errors/warnings reported by cppcheck:
* memory leaks
* resource leaks
* added const's to getters
* removed a few reundant conditions


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@35433 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Resolve further warnings on GCC4.
* Enable -Werror on GCC4 builds as well (limited to the same selected targets).


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@29118 a95241bf-73f2-0310-859d-f6bbb57e9c96

* If only files are watched, PathHandler::_AddFile() will now notify the target
when it was called from a notification function.
* This means you'll no longer have to watch (and scan) directories when you're
interested in new files alone. Maybe this functionality could be used for
other cases as well, or be activated only via an additional flag.
Opinions? Stippi? :-)
* TRACE() no goes to /dev/dprintf by default, added a bit more trace output.
* Automatic whitespace cleanup.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28237 a95241bf-73f2-0310-859d-f6bbb57e9c96

Added clarifying comment for something that puzzled me at first.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26946 a95241bf-73f2-0310-859d-f6bbb57e9c96

* I misunderstood the previous of adding fPath.Path() to the message. It was
meant to be able to tell which monitor the message came from. So I added it
back in. Since I used "path" already for the entry, I renamed the field to
"watched_path". No code was using it yet, so this should be ok and the
purpose more clear anyways.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26945 a95241bf-73f2-0310-859d-f6bbb57e9c96

* TRACE macro no longer needs double parenthesis.
* The global BPathMonitor looper is now always used, no more optional looper
and no more BApplication looper usage. This way we know how the looper behaves
and PathHandler::Quit() can be synchronous. In the end, the bug I was
observing was not caused by the previous asynchronous node monitor stopping,
but this should be safer anyways. When BPathMonitor::StopWatching() returns,
you have really stopped watching and not some time later.
* Introduced "FileEntry" which is an entry_ref plus node id. This is now used
instead of the node_ref for the "watched files set". The whole point
is to really be able to add the "path" field to the B_PATH_MONITOR message.
Previously, the initial path that was passed to StartWatching() was added,
regardless if the message was for an entry somewhere down the hierarchy when
watching recursively. The downside of the new method is that it uses a lot
more RAM per entry. Another option would be to store the node id of the parent
directory and iterate the directory always when in need to construct the path.
* Watching a folder recursively now really adds all the existing subfolders
as well as all the files if not watching for folders only. The tests for the
old implementation only tested what happens when the watched folder was newly
created and then subfolders were created. Those where already added by the
code. Now it also adds the subfolders of folder that appear in a watched
folder.
TODO: Remove folders and files recursively when they dissappear. More testing
for B_ENTRY_MOVED. Optimizations are possible when some information is
retrieved twice. I am also planning to add a way for the BPathMonitor user to
filter the automatically watched files/folders in B_WATCH_RECURSIVELY mode.

I grepped the entire Haiku tree for usage of BPathMonitor. Only net_server
and Mail were using it, but both in a way that is not affected by these
changes. Anyways, TextSearch works more reliable now, even for entries in
subfolders.

Feedback very welcome! :-)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26936 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Rename the private structs to use the same scheme as for classes, which seems
to be the habbit now.
* Improve PathHandler::Quit() to delete the object in case the BMessenger was
not valid or failed to send the Quit message. This should handle the corner
case that the PathHandler's looper was already Quit(). There could still be
a race condition, although I don't know if it affects local message targets
in Haiku. I added a TODO note for that, but I believe it can be neglected.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26877 a95241bf-73f2-0310-859d-f6bbb57e9c96

* StartWatching() now takes an optional BLooper pointer. This looper will
then be used for receiving node monitoring messages.
* Reenabled using be_app as default BLooper if the API user does not provide
one. I think the problem that Stefano needed to work aroung in r23995 was
actually caused by the incorrect locking (an never unlocking) of the looper
before calling PathHandler::Quit().
->If I understand correctly, this code as supposed to work around the possible
situation that the looper holding those PathHandlers may have already quit,
leaving stale PathHandler pointers behind. But that case was not prevented
by the old code anyways, since one would have had to access freed memory to
even get the stale BLooper pointer. The real fix would be to store the
BLooper pointer with each PathHandler so that the possible gone-ness of
those loopers could be checked independent of accessing the PathHandler
pointer. (The whole problem is that PathHandler adds itself to the BLooper
and if the looper quits, it will free all its attached handlers.)
* Introduced a global fallback BLooper for the case that no BApplication is
running, which resolves a TODO.

All this is yet untested, but should have a good chance of working.
(Famous last words...)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26843 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Move the LockLooper() call before PathHandler::Quit() into Quit().
* The B_QUIT_REQUESTED message never arrived for me unless I unlock the
BLooper again, then it works as expected.
* The B_QUIT_REQUESTED handling accessed fOwnsLooper after deleting the
object.
(Review much welcome - I don't understand the purpose of locking the BLooper
at all before trying to use a BMessenger to send it a message.)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26805 a95241bf-73f2-0310-859d-f6bbb57e9c96

Don't choke in BPathMonitor::StopWatch(...) if the client never called
StartWatching() before and the BPathMonitor stuff is therefor not initialized.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26792 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Use new header layout in PathMonitor.h
* Honor 80 char/line limit in PathMonitor.cpp


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26788 a95241bf-73f2-0310-859d-f6bbb57e9c96

We can't use the be_app looper, since it can be already locked from
another thread (for example, when quitting the app/window), and that
would cause a deadlock. Fixes bug #1645.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@23995 a95241bf-73f2-0310-859d-f6bbb57e9c96

* actually initialize the static sLocker with NULL too...


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22700 a95241bf-73f2-0310-859d-f6bbb57e9c96

gcc4 compile fix, but C++ predicates are ugly


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21579 a95241bf-73f2-0310-859d-f6bbb57e9c96

gcc4 fix


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21578 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Added a work-around if the file to be watched does actually exist when starting
to watch it.
* Now adds the path to be watched to the update message (not the path of the
file that actually changed, though).
* Made debug output conditionally compiled in when TRACE_PATH_MONITOR is defined.
* Added PathMonitor.cpp to libbe.so


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21554 a95241bf-73f2-0310-859d-f6bbb57e9c96

Fixed various incorrect uses of the erase() method of several STL containers I introduced before. Thanks to Stefano for the hint!


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21332 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Revamped BPathMonitor API as suggested by Ingo - we could rename it to BNodeMonitor
and add wrappers for watch_node() as well, though.
* Implemented more or less all what is needed for the path monitoring to work.
* Added a test application: works fine under Haiku, but somewhat flaky under BeOS,
dunno why yet.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21066 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Implemented PathMonitor class - untested, but compiles.
* The API is just a proposal at this time, please comment.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21020 a95241bf-73f2-0310-859d-f6bbb57e9c96

PathMonitor: check for NULL before dereferencing.

* should fix #10277.

Fix Coverity CID 1108465: Use after free

Signed-off-by: Ingo Weinhold <ingo_weinhold@gmx.de>

BPathMonitor: add "from path" to B_ENTRY_MOVED notifications

BPathMonitor: rewrite

This resolves all issues the test suite uncovered. It should also deal
with hard links correctly, though that hasn't been tested. Still
unsupported are:
* changes due to mounting/unmounting a volume,
* tracking of symlinks in the path components.

BPathMonitor: pass BMessenger by reference

B_WATCH_FOLDERS_ONLY -> B_WATCH_DIRECTORIES_ONLY

Stick to the nomenclature generally used in the public API.

BPathMonitor: PathHandler::_NotifyTarget(): simplify

* Add optional entry_ref return parameter to _HasFile().
* Simplify _NotifyTarget() by using _HasDirectory() and _HasFile().

BPathMonitor: make the node watching mechanism configurable

Add inner class BWatchingInterface and method SetWatchingInterface().
This abstracts the calls to watch_node() and stop_watching(), thus
making it possible to use the path monitor in Tracker.

BPathMonitor: use pthread_once for initialization

PathMonitor.cpp: some style cleanup

Drop into debugger if the looper can't be locked.
This gets triggered randomly by net_server.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@39934 a95241bf-73f2-0310-859d-f6bbb57e9c96

Resolved TODO about recursively removing watched files and folders when a
directory is moved outside the watched hierarchy or deleted entirely. It
involves another TODO, because the generated notifications are B_ENTRY_REMOVED,
while B_ENTRY_MOVED is probably more correct.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@37049 a95241bf-73f2-0310-859d-f6bbb57e9c96

Fixed various errors/warnings reported by cppcheck:
* memory leaks
* resource leaks
* added const's to getters
* removed a few reundant conditions


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@35433 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Resolve further warnings on GCC4.
* Enable -Werror on GCC4 builds as well (limited to the same selected targets).


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@29118 a95241bf-73f2-0310-859d-f6bbb57e9c96

* If only files are watched, PathHandler::_AddFile() will now notify the target
when it was called from a notification function.
* This means you'll no longer have to watch (and scan) directories when you're
interested in new files alone. Maybe this functionality could be used for
other cases as well, or be activated only via an additional flag.
Opinions? Stippi? :-)
* TRACE() no goes to /dev/dprintf by default, added a bit more trace output.
* Automatic whitespace cleanup.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28237 a95241bf-73f2-0310-859d-f6bbb57e9c96

Added clarifying comment for something that puzzled me at first.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26946 a95241bf-73f2-0310-859d-f6bbb57e9c96

* I misunderstood the previous of adding fPath.Path() to the message. It was
meant to be able to tell which monitor the message came from. So I added it
back in. Since I used "path" already for the entry, I renamed the field to
"watched_path". No code was using it yet, so this should be ok and the
purpose more clear anyways.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26945 a95241bf-73f2-0310-859d-f6bbb57e9c96

* TRACE macro no longer needs double parenthesis.
* The global BPathMonitor looper is now always used, no more optional looper
and no more BApplication looper usage. This way we know how the looper behaves
and PathHandler::Quit() can be synchronous. In the end, the bug I was
observing was not caused by the previous asynchronous node monitor stopping,
but this should be safer anyways. When BPathMonitor::StopWatching() returns,
you have really stopped watching and not some time later.
* Introduced "FileEntry" which is an entry_ref plus node id. This is now used
instead of the node_ref for the "watched files set". The whole point
is to really be able to add the "path" field to the B_PATH_MONITOR message.
Previously, the initial path that was passed to StartWatching() was added,
regardless if the message was for an entry somewhere down the hierarchy when
watching recursively. The downside of the new method is that it uses a lot
more RAM per entry. Another option would be to store the node id of the parent
directory and iterate the directory always when in need to construct the path.
* Watching a folder recursively now really adds all the existing subfolders
as well as all the files if not watching for folders only. The tests for the
old implementation only tested what happens when the watched folder was newly
created and then subfolders were created. Those where already added by the
code. Now it also adds the subfolders of folder that appear in a watched
folder.
TODO: Remove folders and files recursively when they dissappear. More testing
for B_ENTRY_MOVED. Optimizations are possible when some information is
retrieved twice. I am also planning to add a way for the BPathMonitor user to
filter the automatically watched files/folders in B_WATCH_RECURSIVELY mode.

I grepped the entire Haiku tree for usage of BPathMonitor. Only net_server
and Mail were using it, but both in a way that is not affected by these
changes. Anyways, TextSearch works more reliable now, even for entries in
subfolders.

Feedback very welcome! :-)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26936 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Rename the private structs to use the same scheme as for classes, which seems
to be the habbit now.
* Improve PathHandler::Quit() to delete the object in case the BMessenger was
not valid or failed to send the Quit message. This should handle the corner
case that the PathHandler's looper was already Quit(). There could still be
a race condition, although I don't know if it affects local message targets
in Haiku. I added a TODO note for that, but I believe it can be neglected.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26877 a95241bf-73f2-0310-859d-f6bbb57e9c96

* StartWatching() now takes an optional BLooper pointer. This looper will
then be used for receiving node monitoring messages.
* Reenabled using be_app as default BLooper if the API user does not provide
one. I think the problem that Stefano needed to work aroung in r23995 was
actually caused by the incorrect locking (an never unlocking) of the looper
before calling PathHandler::Quit().
->If I understand correctly, this code as supposed to work around the possible
situation that the looper holding those PathHandlers may have already quit,
leaving stale PathHandler pointers behind. But that case was not prevented
by the old code anyways, since one would have had to access freed memory to
even get the stale BLooper pointer. The real fix would be to store the
BLooper pointer with each PathHandler so that the possible gone-ness of
those loopers could be checked independent of accessing the PathHandler
pointer. (The whole problem is that PathHandler adds itself to the BLooper
and if the looper quits, it will free all its attached handlers.)
* Introduced a global fallback BLooper for the case that no BApplication is
running, which resolves a TODO.

All this is yet untested, but should have a good chance of working.
(Famous last words...)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26843 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Move the LockLooper() call before PathHandler::Quit() into Quit().
* The B_QUIT_REQUESTED message never arrived for me unless I unlock the
BLooper again, then it works as expected.
* The B_QUIT_REQUESTED handling accessed fOwnsLooper after deleting the
object.
(Review much welcome - I don't understand the purpose of locking the BLooper
at all before trying to use a BMessenger to send it a message.)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26805 a95241bf-73f2-0310-859d-f6bbb57e9c96

Don't choke in BPathMonitor::StopWatch(...) if the client never called
StartWatching() before and the BPathMonitor stuff is therefor not initialized.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26792 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Use new header layout in PathMonitor.h
* Honor 80 char/line limit in PathMonitor.cpp


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26788 a95241bf-73f2-0310-859d-f6bbb57e9c96

We can't use the be_app looper, since it can be already locked from
another thread (for example, when quitting the app/window), and that
would cause a deadlock. Fixes bug #1645.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@23995 a95241bf-73f2-0310-859d-f6bbb57e9c96

* actually initialize the static sLocker with NULL too...


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22700 a95241bf-73f2-0310-859d-f6bbb57e9c96

gcc4 compile fix, but C++ predicates are ugly


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21579 a95241bf-73f2-0310-859d-f6bbb57e9c96

gcc4 fix


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21578 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Added a work-around if the file to be watched does actually exist when starting
to watch it.
* Now adds the path to be watched to the update message (not the path of the
file that actually changed, though).
* Made debug output conditionally compiled in when TRACE_PATH_MONITOR is defined.
* Added PathMonitor.cpp to libbe.so


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21554 a95241bf-73f2-0310-859d-f6bbb57e9c96

Fixed various incorrect uses of the erase() method of several STL containers I introduced before. Thanks to Stefano for the hint!


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21332 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Revamped BPathMonitor API as suggested by Ingo - we could rename it to BNodeMonitor
and add wrappers for watch_node() as well, though.
* Implemented more or less all what is needed for the path monitoring to work.
* Added a test application: works fine under Haiku, but somewhat flaky under BeOS,
dunno why yet.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21066 a95241bf-73f2-0310-859d-f6bbb57e9c96

* Implemented PathMonitor class - untested, but compiles.
* The API is just a proposal at this time, please comment.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21020 a95241bf-73f2-0310-859d-f6bbb57e9c96