From d9d42ec6bf19ccbd0a89c1ff9c60d98cb50f161d Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Sat, 11 May 2013 18:48:29 -0400 Subject: [PATCH 1/8] Fix case of incorrect return value detection. When using step over to step out of a function, we need to use the address range of the step statement to determine the function which returned said value, not the current IP, as that has already exited the function and will consequently be that of the caller, leading to such returns being attributed to the wrong function, and consequently also the wrong type. --- src/apps/debugger/controllers/ThreadHandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apps/debugger/controllers/ThreadHandler.cpp b/src/apps/debugger/controllers/ThreadHandler.cpp index 72e519b70b..ebc4247e9d 100644 --- a/src/apps/debugger/controllers/ThreadHandler.cpp +++ b/src/apps/debugger/controllers/ThreadHandler.cpp @@ -730,8 +730,8 @@ ThreadHandler::_HandleSingleStepStep(CpuState* cpuState) TRACE_CONTROL("ThreadHandler::_HandleSingleStepStep() " " - adding return value for STEP_OVER\n"); ReturnValueInfo* info = new(std::nothrow) - ReturnValueInfo(cpuState->InstructionPointer(), - cpuState); + ReturnValueInfo(fStepStatement + ->CoveringAddressRange().Start(), cpuState); if (info == NULL) return false; BReference infoReference(info, true); From 3f3ade62237098ec2ecc288111c0d75463ff9091 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Sat, 11 May 2013 19:08:09 -0400 Subject: [PATCH 2/8] Fix incorrect return value problem in STEP_OUT. A similar problem to that described in my previous commit afflicted the step out case as well. We now store the current IP when issuing a step out, and use that as the function address once execution returns. --- src/apps/debugger/controllers/ThreadHandler.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/apps/debugger/controllers/ThreadHandler.cpp b/src/apps/debugger/controllers/ThreadHandler.cpp index ebc4247e9d..5b79d148a5 100644 --- a/src/apps/debugger/controllers/ThreadHandler.cpp +++ b/src/apps/debugger/controllers/ThreadHandler.cpp @@ -269,6 +269,7 @@ ThreadHandler::HandleThreadAction(uint32 action) TRACE_CONTROL(" ip: %#" B_PRIx64 "\n", frame->InstructionPointer()); + target_addr_t frameIP = frame->GetCpuState()->InstructionPointer(); // When the thread is in a syscall, do the same for all step kinds: Stop it // when it returns by means of a breakpoint. if (frame->Type() == STACK_FRAME_TYPE_SYSCALL) { @@ -284,15 +285,14 @@ ThreadHandler::HandleThreadAction(uint32 action) // The second issue is that the temporary breakpoint is probably not necessary // anymore, since single-stepping over "syscall" instructions should just work // as expected. - status_t error = _InstallTemporaryBreakpoint( - frame->GetCpuState()->InstructionPointer()); + status_t error = _InstallTemporaryBreakpoint(frameIP); if (error != B_OK) { _StepFallback(); return; } fStepMode = STEP_OUT; - _RunThread(frame->GetCpuState()->InstructionPointer()); + _RunThread(frameIP); return; } @@ -303,9 +303,10 @@ ThreadHandler::HandleThreadAction(uint32 action) _StepFallback(); return; } + fPreviousInstructionPointer = frameIP; fPreviousFrameAddress = frame->FrameAddress(); fStepMode = STEP_OUT; - _RunThread(frame->GetCpuState()->InstructionPointer()); + _RunThread(frameIP); return; } @@ -324,7 +325,7 @@ ThreadHandler::HandleThreadAction(uint32 action) if (action == MSG_THREAD_STEP_INTO) { // step into fStepMode = STEP_INTO; - _SingleStepThread(frame->GetCpuState()->InstructionPointer()); + _SingleStepThread(frameIP); } else { fPreviousFrameAddress = frame->FrameAddress(); // step over @@ -648,7 +649,7 @@ ThreadHandler::_HandleBreakpointHitStep(CpuState* cpuState) " - step out adding return value\n", cpuState ->StackFramePointer(), fPreviousFrameAddress); ReturnValueInfo* info = new(std::nothrow) ReturnValueInfo( - cpuState->InstructionPointer(), cpuState); + fPreviousInstructionPointer, cpuState); if (info == NULL) return false; BReference infoReference(info, true); From 7daf375812ce3e9ee44b1d09d2c64f0d3ebbaa7c Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 12 May 2013 14:35:01 +0200 Subject: [PATCH 3/8] Terminal: HyperLink: Remove base address, add text property * The base address is no longer used (it was in a an earlier, never committed version), so we can as remove it. * Introduce a text property. --- src/apps/terminal/HyperLink.cpp | 14 +++++++++++--- src/apps/terminal/HyperLink.h | 9 +++++---- src/apps/terminal/TermViewStates.cpp | 3 +-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/apps/terminal/HyperLink.cpp b/src/apps/terminal/HyperLink.cpp index 97e294f7eb..e3eab2843a 100644 --- a/src/apps/terminal/HyperLink.cpp +++ b/src/apps/terminal/HyperLink.cpp @@ -20,11 +20,19 @@ HyperLink::HyperLink() } -HyperLink::HyperLink(const BString& address, Type type, - const BString& baseAddress) +HyperLink::HyperLink(const BString& address, Type type) : + fText(address), + fAddress(address), + fType(type) +{ +} + + +HyperLink::HyperLink(const BString& text, const BString& address, Type type) + : + fText(text), fAddress(address), - fBaseAddress(baseAddress.IsEmpty() ? address : baseAddress), fType(type) { } diff --git a/src/apps/terminal/HyperLink.h b/src/apps/terminal/HyperLink.h index 735c84c50f..ba7d58a13e 100644 --- a/src/apps/terminal/HyperLink.h +++ b/src/apps/terminal/HyperLink.h @@ -20,20 +20,21 @@ public: public: HyperLink(); - HyperLink(const BString& address, Type type, - const BString& baseAddress = BString()); + HyperLink(const BString& address, Type type); + HyperLink(const BString& text, + const BString& address, Type type); bool IsValid() const { return !fAddress.IsEmpty(); } + const BString& Text() const { return fText; } const BString& Address() const { return fAddress; } - const BString& BaseAddress() const { return fBaseAddress; } Type GetType() const { return fType; } status_t Open(); private: + BString fText; BString fAddress; - BString fBaseAddress; Type fType; }; diff --git a/src/apps/terminal/TermViewStates.cpp b/src/apps/terminal/TermViewStates.cpp index 9591983adf..eb190edb27 100644 --- a/src/apps/terminal/TermViewStates.cpp +++ b/src/apps/terminal/TermViewStates.cpp @@ -836,8 +836,7 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, _link = HyperLink(text, i == 0 ? HyperLink::TYPE_PATH_WITH_LINE - : HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN, - path); + : HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN); return true; } } From a15966b3b028d16da29c6b5b7efa43937b6dc7e6 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 12 May 2013 15:21:40 +0200 Subject: [PATCH 4/8] Terminal: hyper link mode: handle relative paths better * We were trying relative paths as is, which means checking them with Terminal's current working directory. Now we use the CWD of the active process. * In case the path is relative, add a context menu item "Copy absolute path". --- src/apps/terminal/TermViewStates.cpp | 64 ++++++++++++++++++++++------ src/apps/terminal/TermViewStates.h | 3 ++ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/apps/terminal/TermViewStates.cpp b/src/apps/terminal/TermViewStates.cpp index eb190edb27..6176de4196 100644 --- a/src/apps/terminal/TermViewStates.cpp +++ b/src/apps/terminal/TermViewStates.cpp @@ -30,6 +30,7 @@ #include #include +#include "ActiveProcessInfo.h" #include "Shell.h" #include "TermConst.h" #include "TerminalBuffer.h" @@ -52,6 +53,7 @@ static const uint32 kAutoScroll = 'AScr'; static const uint32 kMessageOpenLink = 'OLnk'; static const uint32 kMessageCopyLink = 'CLnk'; +static const uint32 kMessageCopyAbsolutePath = 'CAbs'; static const uint32 kMessageMenuClosed = 'MClo'; @@ -638,6 +640,7 @@ TermView::HyperLinkState::HyperLinkState(TermView* view) fURLCharClassifier(kURLAdditionalWordCharacters), fPathComponentCharClassifier( BString(kDefaultAdditionalWordCharacters).RemoveFirst("/")), + fCurrentDirectory(), fHighlight(), fHighlightActive(false) { @@ -648,6 +651,12 @@ TermView::HyperLinkState::HyperLinkState(TermView* view) void TermView::HyperLinkState::Entered() { + ActiveProcessInfo activeProcessInfo; + if (fView->GetActiveProcessInfo(activeProcessInfo)) + fCurrentDirectory = activeProcessInfo.CurrentDirectory(); + else + fCurrentDirectory.Truncate(0); + _UpdateHighlight(); } @@ -792,9 +801,9 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, return false; // check, whether the file exists - struct stat st; - if (lstat(text, &st) == 0) { - _link = HyperLink(text, HyperLink::TYPE_PATH); + BString actualPath; + if (_EntryExists(text, actualPath)) { + _link = HyperLink(text, actualPath, HyperLink::TYPE_PATH); return true; } @@ -813,8 +822,8 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, if (!textBuffer->PreviousLinePos(_end)) return false; - if (lstat(text, &st) == 0) { - _link = HyperLink(text, HyperLink::TYPE_PATH); + if (_EntryExists(text, actualPath)) { + _link = HyperLink(text, actualPath, HyperLink::TYPE_PATH); return true; } } @@ -832,8 +841,10 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, return false; path.Truncate(colonIndex); - if (lstat(path, &st) == 0) { - _link = HyperLink(text, + if (_EntryExists(path, actualPath)) { + BString address = path == actualPath + ? text : BString(fCurrentDirectory) << '/' << text; + _link = HyperLink(text, address, i == 0 ? HyperLink::TYPE_PATH_WITH_LINE : HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN); @@ -845,6 +856,25 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, } +bool +TermView::HyperLinkState::_EntryExists(const BString& path, + BString& _actualPath) const +{ + if (path.IsEmpty()) + return false; + + if (path[0] == '/' || fCurrentDirectory.IsEmpty()) { + _actualPath = path; + } else { + _actualPath.Truncate(0); + _actualPath << fCurrentDirectory << '/' << path; + } + + struct stat st; + return lstat(_actualPath, &st) == 0; +} + + void TermView::HyperLinkState::_UpdateHighlight() { @@ -947,9 +977,12 @@ TermView::HyperLinkMenuState::Prepare(BPoint point, const HyperLink& link) case HyperLink::TYPE_PATH: case HyperLink::TYPE_PATH_WITH_LINE: case HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN: - menuBuilder - .AddItem(B_TRANSLATE("Open path"), kMessageOpenLink) - .AddItem(B_TRANSLATE("Copy path"), kMessageCopyLink); + menuBuilder.AddItem(B_TRANSLATE("Open path"), kMessageOpenLink); + menuBuilder.AddItem(B_TRANSLATE("Copy path"), kMessageCopyLink); + if (fLink.Text() != fLink.Address()) { + menuBuilder.AddItem(B_TRANSLATE("Copy absolute path"), + kMessageCopyAbsolutePath); + } break; } menu->SetTargetForItems(fView); @@ -974,22 +1007,27 @@ TermView::HyperLinkMenuState::MessageReceived(BMessage* message) return true; case kMessageCopyLink: + case kMessageCopyAbsolutePath: + { if (fLink.IsValid()) { + BString toCopy = message->what == kMessageCopyLink + ? fLink.Text() : fLink.Address(); + if (!be_clipboard->Lock()) return true; be_clipboard->Clear(); if (BMessage *data = be_clipboard->Data()) { - data->AddData("text/plain", B_MIME_TYPE, - fLink.Address().String(), - fLink.Address().Length()); + data->AddData("text/plain", B_MIME_TYPE, toCopy.String(), + toCopy.Length()); be_clipboard->Commit(); } be_clipboard->Unlock(); } return true; + } case kMessageMenuClosed: fView->_NextState(fView->fDefaultState); diff --git a/src/apps/terminal/TermViewStates.h b/src/apps/terminal/TermViewStates.h index af8e2955a8..7fef419449 100644 --- a/src/apps/terminal/TermViewStates.h +++ b/src/apps/terminal/TermViewStates.h @@ -132,6 +132,8 @@ private: bool _GetHyperLinkAt(BPoint where, bool pathPrefixOnly, HyperLink& _link, TermPos& _start, TermPos& _end); + bool _EntryExists(const BString& path, + BString& _actualPath) const; void _UpdateHighlight(); void _UpdateHighlight(BPoint where, int32 modifiers); @@ -142,6 +144,7 @@ private: private: DefaultCharClassifier fURLCharClassifier; DefaultCharClassifier fPathComponentCharClassifier; + BString fCurrentDirectory; TermViewHighlight fHighlight; bool fHighlightActive; }; From 78a1163c7b5d51d09f757f125ccd683bef0a06d9 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 12 May 2013 17:17:46 +0200 Subject: [PATCH 5/8] Terminal: hyper link mode: Try more aggressively to detect a path Consider ':' a potential path delimiter and try all combinations of chopped off prefixes and suffixes. This makes detection in the output of a multi-file grep work even if the found line starts with a path character or is a path. A path in the typical colon delimited search paths (e.g. PATH) is detected as well. --- src/apps/terminal/TermViewStates.cpp | 139 +++++++++++++++++++-------- src/apps/terminal/TermViewStates.h | 6 ++ 2 files changed, 103 insertions(+), 42 deletions(-) diff --git a/src/apps/terminal/TermViewStates.cpp b/src/apps/terminal/TermViewStates.cpp index 6176de4196..e0f33b24ee 100644 --- a/src/apps/terminal/TermViewStates.cpp +++ b/src/apps/terminal/TermViewStates.cpp @@ -30,6 +30,8 @@ #include #include +#include + #include "ActiveProcessInfo.h" #include "Shell.h" #include "TermConst.h" @@ -800,55 +802,108 @@ TermView::HyperLinkState::_GetHyperLinkAt(BPoint where, bool pathPrefixOnly, if (text.IsEmpty()) return false; - // check, whether the file exists - BString actualPath; - if (_EntryExists(text, actualPath)) { - _link = HyperLink(text, actualPath, HyperLink::TYPE_PATH); - return true; - } - - // As such this isn't an existing path. Try a few common alternative cases: - // * ":" - // * ":" - // * "::" - // * "::" - // * ":::" - - if (text.Length() <= 1) - return false; - - if (text[text.Length() - 1] == ':') { - text.Truncate(text.Length() - 1); - if (!textBuffer->PreviousLinePos(_end)) + // Collect a list of colons in the string and their respective positions in + // the text buffer. We do this up-front so we can unlock the text buffer + // while we're doing all the entry existence tests. + typedef Array ColonList; + ColonList colonPositions; + TermPos searchPos = _start; + for (int32 index = 0; (index = text.FindFirst(':', index)) >= 0;) { + TermPos foundStart; + TermPos foundEnd; + if (!textBuffer->Find(":", searchPos, true, true, false, foundStart, + foundEnd)) { return false; - - if (_EntryExists(text, actualPath)) { - _link = HyperLink(text, actualPath, HyperLink::TYPE_PATH); - return true; } + + CharPosition colonPosition; + colonPosition.index = index; + colonPosition.position = foundStart; + if (!colonPositions.Add(colonPosition)) + return false; + + index++; + searchPos = foundEnd; } - BString path = text; + textBufferLocker.Unlock(); - for (int32 i = 0; i < 2; i++) { - int32 colonIndex = path.FindLast(':'); - if (colonIndex <= 0 || colonIndex == path.Length() - 1) - return false; + // Since we also want to consider ':' a potential path delimiter, in two + // nested loops we chop off components from the beginning respective the + // end. + BString originalText = text; + TermPos originalStart = _start; + TermPos originalEnd = _end; - char* numberEnd; - strtol(path.String() + colonIndex + 1, &numberEnd, 0); - if (*numberEnd != '\0') - return false; + int32 colonCount = colonPositions.Count(); + for (int32 startColonIndex = -1; startColonIndex < colonCount; + startColonIndex++) { + int32 startIndex; + if (startColonIndex < 0) { + startIndex = 0; + _start = originalStart; + } else { + startIndex = colonPositions[startColonIndex].index + 1; + _start = colonPositions[startColonIndex].position; + if (_start >= pos) + break; + _start.x++; + // Note: This is potentially a non-normalized position (i.e. + // the end of a soft-wrapped line). While not that nice, it + // works anyway. + } - path.Truncate(colonIndex); - if (_EntryExists(path, actualPath)) { - BString address = path == actualPath - ? text : BString(fCurrentDirectory) << '/' << text; - _link = HyperLink(text, address, - i == 0 - ? HyperLink::TYPE_PATH_WITH_LINE - : HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN); - return true; + for (int32 endColonIndex = colonCount; endColonIndex > startColonIndex; + endColonIndex--) { + int32 endIndex; + if (endColonIndex == colonCount) { + endIndex = originalText.Length(); + _end = originalEnd; + } else { + endIndex = colonPositions[endColonIndex].index; + _end = colonPositions[endColonIndex].position; + if (_end <= pos) + break; + } + + originalText.CopyInto(text, startIndex, endIndex - startIndex); + if (text.IsEmpty()) + continue; + + // check, whether the file exists + BString actualPath; + if (_EntryExists(text, actualPath)) { + _link = HyperLink(text, actualPath, HyperLink::TYPE_PATH); + return true; + } + + // As such this isn't an existing path. We also want to recognize: + // * ":" + // * "::" + + BString path = text; + + for (int32 i = 0; i < 2; i++) { + int32 colonIndex = path.FindLast(':'); + if (colonIndex <= 0 || colonIndex == path.Length() - 1) + break; + + char* numberEnd; + strtol(path.String() + colonIndex + 1, &numberEnd, 0); + if (*numberEnd != '\0') + break; + + path.Truncate(colonIndex); + if (_EntryExists(path, actualPath)) { + BString address = path == actualPath + ? text : BString(fCurrentDirectory) << '/' << text; + _link = HyperLink(text, address, + i == 0 + ? HyperLink::TYPE_PATH_WITH_LINE + : HyperLink::TYPE_PATH_WITH_LINE_AND_COLUMN); + return true; + } + } } } diff --git a/src/apps/terminal/TermViewStates.h b/src/apps/terminal/TermViewStates.h index 7fef419449..106f2af195 100644 --- a/src/apps/terminal/TermViewStates.h +++ b/src/apps/terminal/TermViewStates.h @@ -128,6 +128,12 @@ private: virtual rgb_color BackgroundColor(); virtual uint32 AdjustTextAttributes(uint32 attributes); +private: + struct CharPosition { + int32 index; + TermPos position; + }; + private: bool _GetHyperLinkAt(BPoint where, bool pathPrefixOnly, HyperLink& _link, From bb4537c8412181c830e790cf9498c4b4441f617a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Duval?= Date: Sun, 12 May 2013 16:34:12 +0200 Subject: [PATCH 6/8] JPEGTranslator: fixes a 64 bit warning --- src/add-ons/translators/jpeg/JPEGTranslator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/add-ons/translators/jpeg/JPEGTranslator.cpp b/src/add-ons/translators/jpeg/JPEGTranslator.cpp index c6bb97b963..ab57fcb88e 100644 --- a/src/add-ons/translators/jpeg/JPEGTranslator.cpp +++ b/src/add-ons/translators/jpeg/JPEGTranslator.cpp @@ -402,7 +402,7 @@ SSlider::SSlider(const char* name, const char* label, const char* SSlider::UpdateText() const { - snprintf(fStatusLabel, sizeof(fStatusLabel), "%ld", Value()); + snprintf(fStatusLabel, sizeof(fStatusLabel), "%" B_PRId32, Value()); return fStatusLabel; } From e431f228752218cb258ab00db7e4c390f8f0184d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Duval?= Date: Sun, 12 May 2013 18:46:43 +0200 Subject: [PATCH 7/8] playground: fixes a 64 bit warning. --- src/tests/servers/app/playground/ObjectWindow.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tests/servers/app/playground/ObjectWindow.cpp b/src/tests/servers/app/playground/ObjectWindow.cpp index 0b6b9cc590..e78462287a 100644 --- a/src/tests/servers/app/playground/ObjectWindow.cpp +++ b/src/tests/servers/app/playground/ObjectWindow.cpp @@ -92,8 +92,9 @@ class ObjectListView : public BListView { virtual bool InitiateDrag(BPoint point, int32 itemIndex, bool wasSelected) { - printf("InitiateDrag(BPoint(%.1f, %.1f), itemIndex: %ld, wasSelected: %d)\n", - point.x, point.y, itemIndex, wasSelected); + printf("InitiateDrag(BPoint(%.1f, %.1f), itemIndex: %" B_PRId32 + ", wasSelected: %d)\n", point.x, point.y, itemIndex, + wasSelected); SwapItems(itemIndex, itemIndex + 1); return true; } From c895d331c9c791de2a1ba9fac405ce9dd2ee3d3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Sun, 12 May 2013 21:05:05 +0200 Subject: [PATCH 8/8] app_server: added an ASSERT to Desktop::_Windows(). * So that something like #9595 should not happen again. --- src/servers/app/Desktop.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/servers/app/Desktop.cpp b/src/servers/app/Desktop.cpp index f9e8d4553c..808ac9272a 100644 --- a/src/servers/app/Desktop.cpp +++ b/src/servers/app/Desktop.cpp @@ -2704,6 +2704,7 @@ Desktop::WindowForClientLooperPort(port_id port) WindowList& Desktop::_Windows(int32 index) { + ASSERT(index >= 0 && index < kMaxWorkspaces); return fWorkspaces[index].Windows(); }