Tracker: Throw exception on assert failure

... in situations where a NULL pointer dereference was vital to the
functioning of the method we use a stronger assert that throws an
exception on failure. This is accomplished by a new method in
Utilities.cpp, ThrowOnAssert().

None of these conditions should ever be true, if they are it means that
the code is written improperly and would have resulted in a NULL
dereference and undefined behavior (most likely a crash) before.

Most instances of ThrowOnAssert() either replace an ASSERT followed
by a dereference or an early return that covered the error.

Also remove _ThrowOnErrorWithMessage() which wasn't being used.
This commit is contained in:
John Scipione
2014-08-10 15:07:38 -04:00
parent 9cc03189fa
commit 2f60dea53a
13 changed files with 82 additions and 75 deletions
+9 -6
View File
@@ -38,6 +38,8 @@ All rights reserved.
#include <Debug.h>
#include <Node.h>
#include "Utilities.h"
// ToDo:
// lazy Rewind from Drive, only if data is available
@@ -309,7 +311,8 @@ AttributeStreamFileNode::SetTo(BNode* node)
off_t
AttributeStreamFileNode::Contains(const char* name, uint32 type)
{
ASSERT(fNode);
ThrowOnAssert(fNode != NULL);
attr_info info;
if (fNode->GetAttrInfo(name, &info) != B_OK)
return 0;
@@ -348,8 +351,7 @@ off_t
AttributeStreamFileNode::Write(const char* name, const char* foreignName,
uint32 type, off_t size, const void* buffer)
{
ASSERT(fNode != NULL);
ASSERT(dynamic_cast<BNode*>(fNode) != NULL);
ThrowOnAssert(fNode != NULL);
off_t result = fNode->WriteAttr(name, type, 0, buffer, (size_t)size);
if (result == size && foreignName != NULL) {
@@ -365,10 +367,11 @@ AttributeStreamFileNode::Write(const char* name, const char* foreignName,
bool
AttributeStreamFileNode::Drive()
{
ASSERT(fNode != NULL);
if (!_inherited::Drive())
return false;
ThrowOnAssert(fNode != NULL);
const AttributeInfo* attr;
while ((attr = fReadFrom->Next()) != 0) {
const char* data = fReadFrom->Get();
@@ -395,7 +398,7 @@ AttributeStreamFileNode::Get()
bool
AttributeStreamFileNode::Fill(char* buffer) const
{
ASSERT(fNode != NULL);
ThrowOnAssert(fNode != NULL);
return fNode->ReadAttr(fCurrentAttr.Name(), fCurrentAttr.Type(), 0,
buffer, (size_t)fCurrentAttr.Size()) == (ssize_t)fCurrentAttr.Size();
@@ -405,8 +408,8 @@ AttributeStreamFileNode::Fill(char* buffer) const
const AttributeInfo*
AttributeStreamFileNode::Next()
{
ASSERT(fNode != NULL);
ASSERT(fReadFrom == NULL);
ThrowOnAssert(fNode != NULL);
char attrName[256];
if (fNode->GetNextAttrName(attrName) != B_OK)
+3 -6
View File
@@ -334,8 +334,7 @@ DraggableContainerIcon::MouseDown(BPoint where)
{
// we only like container windows
BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
if (window == NULL)
return;
ThrowOnAssert(window != NULL);
// we don't like the Trash icon (because it cannot be moved)
if (window->IsTrash() || window->IsPrintersDir())
@@ -460,8 +459,7 @@ void
DraggableContainerIcon::FrameMoved(BPoint)
{
BMenuBar* bar = dynamic_cast<BMenuBar*>(Parent());
if (bar == NULL)
return;
ThrowOnAssert(bar != NULL);
// TODO: ugly hack following:
// This is a trick to get the actual width of all menu items
@@ -492,8 +490,7 @@ void
DraggableContainerIcon::Draw(BRect updateRect)
{
BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
if (window == NULL)
return;
ThrowOnAssert(window != NULL);
if (be_control_look != NULL) {
BRect rect(Bounds());
+1 -2
View File
@@ -311,8 +311,7 @@ void
BCountView::MouseDown(BPoint)
{
BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
if (window == NULL)
return;
ThrowOnAssert(window != NULL);
window->Activate();
window->UpdateIfNeeded();
+2 -3
View File
@@ -83,7 +83,7 @@ DesktopPoseView::InitDesktopDirentIterator(BPoseView* nodeMonitoringTarget,
ASSERT(sourceModel.Node() != NULL);
BDirectory* sourceDirectory = dynamic_cast<BDirectory*>(sourceModel.Node());
ASSERT(sourceDirectory != NULL);
ThrowOnAssert(sourceDirectory != NULL);
// build an iterator list, start with boot
EntryListBase* perDesktopIterator
@@ -224,8 +224,7 @@ void
DesktopPoseView::AdaptToVolumeChange(BMessage* message)
{
TTracker* tracker = dynamic_cast<TTracker*>(be_app);
if (tracker == NULL)
return;
ThrowOnAssert(tracker != NULL);
bool showDisksIcon = false;
bool mountVolumesOnDesktop = true;
+4 -4
View File
@@ -1243,8 +1243,7 @@ LowLevelCopy(BEntry* srcEntry, StatStruct* srcStat, BDirectory* destDir,
char linkpath[MAXPATHLEN];
ThrowOnError(srcLink.SetTo(srcEntry));
ThrowIfNotSize(srcLink.ReadLink(linkpath, MAXPATHLEN-1));
ThrowOnError(srcLink.ReadLink(linkpath, MAXPATHLEN - 1));
ThrowOnError(destDir->CreateSymLink(destName, linkpath, &newLink));
node_ref destNodeRef;
@@ -1762,10 +1761,11 @@ MoveItem(BEntry* entry, BDirectory* destDir, BPoint* loc, uint32 moveMode,
return error.fError;
} catch (FailWithAlert error) {
BString buffer(error.fString);
if (error.fName)
if (error.fName != NULL)
buffer.ReplaceFirst("%name", error.fName);
else
buffer << error.fString;
buffer << error.fString;
BAlert* alert = new BAlert("", buffer.String(), B_TRANSLATE("OK"),
0, 0, B_WIDTH_AS_USUAL, B_WARNING_ALERT);
alert->SetFlags(alert->Flags() | B_CLOSE_ON_ESCAPE);
+4
View File
@@ -718,6 +718,8 @@ BNavMenu::BuildVolumeMenu()
int
BNavMenu::CompareFolderNamesFirstOne(const BMenuItem* i1, const BMenuItem* i2)
{
ThrowOnAssert(i1 != NULL && i2 != NULL);
const ModelMenuItem* item1 = dynamic_cast<const ModelMenuItem*>(i1);
const ModelMenuItem* item2 = dynamic_cast<const ModelMenuItem*>(i2);
@@ -733,6 +735,8 @@ BNavMenu::CompareFolderNamesFirstOne(const BMenuItem* i1, const BMenuItem* i2)
int
BNavMenu::CompareOne(const BMenuItem* i1, const BMenuItem* i2)
{
ThrowOnAssert(i1 != NULL && i2 != NULL);
return strcasecmp(i1->Label(), i2->Label());
}
+2
View File
@@ -1560,6 +1560,8 @@ bool
SearchForSignatureEntryList::CanOpenWithFilter(const Model* appModel,
const BMessage* entriesToOpen, const entry_ref* preferredApp)
{
ThrowOnAssert(appModel != NULL);
if (!appModel->IsExecutable() || !appModel->Node()) {
// weed out non-executable
#if xDEBUG
+25 -24
View File
@@ -745,14 +745,16 @@ BPoseView::SavePoseLocations(BRect* frameIfDesktop)
if (!fSavePoseLocations)
return;
ASSERT(TargetModel());
ASSERT(Window()->IsLocked());
Model* targetModel = TargetModel();
ThrowOnAssert(targetModel != NULL);
BVolume volume(TargetModel()->NodeRef()->device);
if (volume.InitCheck() != B_OK)
return;
if (!TargetModel()->IsRoot()
if (!targetModel->IsRoot()
&& (volume.IsReadOnly() || !volume.KnowsAttr())) {
// check that we can write out attrs; Root should always work
// because it gets saved on the boot disk but the above checks
@@ -770,7 +772,7 @@ BPoseView::SavePoseLocations(BRect* frameIfDesktop)
poseInfo.fInvisible = false;
if (model->IsRoot())
poseInfo.fInitedDirectory = TargetModel()->NodeRef()->node;
poseInfo.fInitedDirectory = targetModel->NodeRef()->node;
else
poseInfo.fInitedDirectory = model->EntryRef()->directory;
@@ -1331,18 +1333,10 @@ BPoseView::AddPosesTask(void* castToParams)
thread_id threadID = find_thread(NULL);
BPoseView* view = dynamic_cast<BPoseView*>(lock.Handler());
ASSERT(view != NULL);
if (view == NULL)
return B_ERROR;
ThrowOnAssert(view != NULL);
BWindow* window = dynamic_cast<BWindow*>(lock.Looper());
ASSERT(window != NULL);
if (window == NULL)
return B_ERROR;
ThrowOnAssert(window != NULL);
// allocate the iterator we will use for adding poses; this
// can be a directory or any other collection of entry_refs, such
@@ -3392,7 +3386,7 @@ void
BPoseView::NewFileFromTemplate(const BMessage* message)
{
Model* targetModel = TargetModel();
ASSERT(targetModel != NULL);
ThrowOnAssert(targetModel != NULL);
entry_ref destEntryRef;
node_ref destNodeRef;
@@ -3413,7 +3407,7 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
if (dir.InitCheck() == B_OK) {
// special handling of directories
if (FSCreateNewFolderIn(TargetModel()->NodeRef(), &destEntryRef,
if (FSCreateNewFolderIn(targetModel->NodeRef(), &destEntryRef,
&destNodeRef) == B_OK) {
BEntry destEntry(&destEntryRef);
destEntry.Rename(fileName);
@@ -3452,7 +3446,7 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
// start renaming the entry
int32 index;
BPose* pose = EntryCreated(TargetModel()->NodeRef(), &destNodeRef,
BPose* pose = EntryCreated(targetModel->NodeRef(), &destNodeRef,
destEntryRef.name, &index);
if (fFiltering) {
@@ -3477,18 +3471,19 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
void
BPoseView::NewFolder(const BMessage* message)
{
ASSERT(TargetModel());
Model* targetModel = TargetModel();
ThrowOnAssert(targetModel != NULL);
entry_ref ref;
node_ref nodeRef;
if (FSCreateNewFolderIn(TargetModel()->NodeRef(), &ref, &nodeRef) == B_OK) {
if (FSCreateNewFolderIn(targetModel->NodeRef(), &ref, &nodeRef) == B_OK) {
// try to place new folder at click point or under mouse if possible
PlaceFolder(&ref, message);
int32 index;
BPose* pose = EntryCreated(TargetModel()->NodeRef(), &nodeRef, ref.name,
BPose* pose = EntryCreated(targetModel->NodeRef(), &nodeRef, ref.name,
&index);
if (fFiltering) {
@@ -4888,10 +4883,12 @@ static bool
AddOneToLaunchMessage(BPose* pose, BPoseView*, void* castToParams)
{
LaunchParams* params = (LaunchParams*)castToParams;
ThrowOnAssert(params != NULL);
ThrowOnAssert(pose != NULL);
ThrowOnAssert(pose->TargetModel() != NULL);
ASSERT(pose->TargetModel());
if (params->app->IsDropTarget(params->checkTypes
? pose->TargetModel() : 0, true)) {
? pose->TargetModel() : NULL, true)) {
params->refsMessage->AddRef("refs", pose->TargetModel()->EntryRef());
}
@@ -5637,7 +5634,7 @@ BPoseView::EntryMoved(const BMessage* message)
}
Model* targetModel = TargetModel();
ASSERT(targetModel != NULL);
ThrowOnAssert(targetModel != NULL);
node_ref thisDirNode;
if (ContainerWindow()->IsTrash()) {
@@ -6097,9 +6094,9 @@ BPoseView::MoveListToTrash(BObjectList<entry_ref>* list, bool selectNext,
pointInPose.y += fListElemHeight * index;
TTracker* tracker = dynamic_cast<TTracker*>(be_app);
if (tracker != NULL) {
ThrowOnAssert(TargetModel() != NULL);
ASSERT(TargetModel());
if (tracker) {
// add a function object to the list of tasks to run
// that will select the next item after the one we just
// deleted
@@ -6320,6 +6317,8 @@ BPoseView::Delete(BObjectList<entry_ref>* list, bool selectNext, bool askUser)
TTracker* tracker = dynamic_cast<TTracker*>(be_app);
if (tracker != NULL) {
ThrowOnAssert(TargetModel() != NULL);
// add a function object to the list of tasks to run
// that will select the next item after the one we just
// deleted
@@ -6361,6 +6360,8 @@ BPoseView::RestoreItemsFromTrash(BObjectList<entry_ref>* list, bool selectNext)
TTracker* tracker = dynamic_cast<TTracker*>(be_app);
if (tracker != NULL) {
ThrowOnAssert(TargetModel() != NULL);
// add a function object to the list of tasks to run
// that will select the next item after the one we just
// restored
+2 -1
View File
@@ -353,7 +353,8 @@ BQueryPoseView::InitDirentIterator(const entry_ref* ref)
delta *= 1000000;
TTracker* tracker = dynamic_cast<TTracker*>(be_app);
ASSERT(tracker);
ThrowOnAssert(tracker != NULL);
tracker->MainTaskLoop()->RunLater(
NewLockingMemberFunctionObject(&BQueryPoseView::Refresh, this),
delta);
+8 -2
View File
@@ -297,8 +297,14 @@ TextViewFilter(BMessage* message, BHandler**, BMessageFilter* filter)
if (message->FindInt8("byte", (int8*)&key) != B_OK)
return B_DISPATCH_MESSAGE;
BPoseView* poseView = dynamic_cast<BContainerWindow*>(
filter->Looper())->PoseView();
ThrowOnAssert(filter != NULL);
BContainerWindow* window = dynamic_cast<BContainerWindow*>(
filter->Looper());
ThrowOnAssert(window != NULL);
BPoseView* poseView = window->PoseView();
ThrowOnAssert(poseView != NULL);
if (key == B_RETURN || key == B_ESCAPE) {
poseView->CommitActivePose(key == B_RETURN);
+10 -8
View File
@@ -1654,12 +1654,15 @@ ComputeTypeAheadScore(const char* text, const char* match, bool wordMode)
}
// #pragma mark - throw on error functions.
void
_ThrowOnError(status_t result, const char* DEBUG_ONLY(file),
int32 DEBUG_ONLY(line))
{
if (result != B_OK) {
PRINT(("failing %s at %s:%d\n", strerror(result), file, (int)line));
PRINT(("%s at %s:%d\n", strerror(result), file, (int)line));
throw result;
}
}
@@ -1670,20 +1673,19 @@ _ThrowIfNotSize(ssize_t size, const char* DEBUG_ONLY(file),
int32 DEBUG_ONLY(line))
{
if (size < B_OK) {
PRINT(("failing %s at %s:%d\n", strerror(size), file, (int)line));
PRINT(("%s at %s:%d\n", strerror((status_t)size), file, (int)line));
throw (status_t)size;
}
}
void
_ThrowOnError(status_t result, const char* DEBUG_ONLY(debugString),
const char* DEBUG_ONLY(file), int32 DEBUG_ONLY(line))
_ThrowOnAssert(bool success, const char* DEBUG_ONLY(file),
int32 DEBUG_ONLY(line))
{
if (result != B_OK) {
PRINT(("failing %s, %s at %s:%d\n", debugString, strerror(result), file,
(int)line));
throw result;
if (!success) {
PRINT(("Assert failed at %s:%d\n", file, (int)line));
throw B_ERROR;
}
}
+7 -9
View File
@@ -474,20 +474,18 @@ ThrowOnInitCheckError(InitCheckable* item)
}
#if DEBUG
# define ThrowOnError(result) _ThrowOnError(result, __FILE__, __LINE__)
# define ThrowIfNotSize(result) _ThrowIfNotSize(result, __FILE__, __LINE__)
# define ThrowOnErrorWithMessage(result, debugStr) \
_ThrowOnError(result, debugStr, __FILE__, __LINE__)
# define ThrowOnError(x) _ThrowOnError(x, __FILE__, __LINE__)
# define ThrowIfNotSize(x) _ThrowIfNotSize(x, __FILE__, __LINE__)
# define ThrowOnAssert(x) _ThrowOnAssert(x, __FILE__, __LINE__)
#else
# define ThrowOnError(x) _ThrowOnError(x, 0, 0)
# define ThrowIfNotSize(x) _ThrowIfNotSize(x, 0, 0)
# define ThrowOnErrorWithMessage(result, debugStr) \
_ThrowOnError(result, debugStr, __FILE__, __LINE__)
# define ThrowOnError(x) _ThrowOnError(x, NULL, 0)
# define ThrowIfNotSize(x) _ThrowIfNotSize(x, NULL, 0)
# define ThrowOnAssert(x) _ThrowOnAssert(x, NULL, 0)
#endif
void _ThrowOnError(status_t, const char*, int32);
void _ThrowIfNotSize(ssize_t, const char*, int32);
void _ThrowOnError(status_t, const char* debugStr, const char*, int32);
void _ThrowOnAssert(bool, const char*, int32);
// stub calls that work around BAppFile info inefficiency
status_t GetAppSignatureFromAttr(BFile*, char*);
+5 -10
View File
@@ -556,8 +556,7 @@ int
StringAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
{
StringAttributeText* compareTo = dynamic_cast<StringAttributeText*>(&attr);
ASSERT(compareTo != NULL);
ThrowOnAssert(compareTo != NULL);
if (fValueDirty)
ReadValue(&fFullValueText);
@@ -647,8 +646,7 @@ int
ScalarAttributeText::Compare(WidgetAttributeText& attr, BPoseView*)
{
ScalarAttributeText* compareTo = dynamic_cast<ScalarAttributeText*>(&attr);
ASSERT(compareTo != NULL);
ThrowOnAssert(compareTo != NULL);
if (fValueDirty)
fValue = ReadValue();
@@ -756,8 +754,7 @@ int
NameAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
{
NameAttributeText* compareTo = dynamic_cast<NameAttributeText*>(&attr);
ASSERT(compareTo != NULL);
ThrowOnAssert(compareTo != NULL);
if (fValueDirty)
ReadValue(&fFullValueText);
@@ -892,8 +889,7 @@ RealNameAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
{
RealNameAttributeText* compareTo
= dynamic_cast<RealNameAttributeText*>(&attr);
ASSERT(compareTo != NULL);
ThrowOnAssert(compareTo != NULL);
if (fValueDirty)
ReadValue(&fFullValueText);
@@ -1567,8 +1563,7 @@ GenericAttributeText::Compare(WidgetAttributeText& attr, BPoseView*)
{
GenericAttributeText* compareTo
= dynamic_cast<GenericAttributeText*>(&attr);
ASSERT(compareTo != NULL);
ThrowOnAssert(compareTo != NULL);
if (fValueDirty)
ReadValue(&fFullValueText);