Npp754 #50
Npp754 #50
Conversation
|
|
||
| void NotepadPlusWrapper::saveFile(boost::python::str filename) | ||
| { | ||
| std::shared_ptr<TCHAR> s = WcharMbcsConverter::char2tchar((const char*)boost::python::extract<const char*>(filename)); |
There was a problem hiding this comment.
Is this behaving correctly with unicode file names?
There was a problem hiding this comment.
Yes, I can open, activate, reload, save etc... without a problem but
notepad.launchFindInFilesDlg seems to be behaving different.
| callbackT m_callbacks; | ||
| bool m_notificationsEnabled; | ||
| HANDLE m_callbackMutex; | ||
|
|
||
| void notAllowedInScintillaCallback(const char *message); | ||
| void notAllowedInScintillaCallback(const char *message); | ||
| void NotepadPlusWrapper::invalidValueProvided(const char *message); |
There was a problem hiding this comment.
NotepadPlusWrapper:: is not necessary here.
| @@ -591,7 +603,14 @@ BufferEncoding NotepadPlusWrapper::getEncoding() | |||
|
|
|||
| BufferEncoding NotepadPlusWrapper::getBufferEncoding(intptr_t bufferID) | |||
| { | |||
| return static_cast<BufferEncoding>(callNotepad(NPPM_GETBUFFERENCODING, static_cast<WPARAM>(bufferID))); | |||
| if (bufferID >= 0) | |||
There was a problem hiding this comment.
Do you have an example which shows the problem with bufferID < 0? Seems negative values are ok. The bufferID is directly used as pointer. There is just no check in N++ that the bufferID is a valid one and therefore an invalid pointer access could happen if the bufferID is unknown.
There was a problem hiding this comment.
For example notepad.reloadBuffer(-1,True) or notepad.getLangType(-1)
There was a problem hiding this comment.
Callstack is:
notepad++.exe!Buffer::setDeferredReload() Zeile 465 C++
notepad++.exe!FileManager::reloadBufferDeferred(Buffer * id) Zeile 681 C++
notepad++.exe!Notepad_plus::doReload(Buffer * id, bool alert) Zeile 448 C++
notepad++.exe!Notepad_plus::process(HWND__ * hwnd, unsigned int message, unsigned __int64 wParam, int64 lParam) Zeile 411 C++
notepad++.exe!Notepad_plus_Window::runProc(HWND * hwnd, unsigned int message, unsigned __int64 wParam, int64 lParam) Zeile 120 C++
notepad++.exe!Notepad_plus_Window::Notepad_plus_Proc(HWND * hwnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Zeile 96 C++
and FileManager::reloadBufferDeferred(BufferID id) at
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/3a52ab1af16df8833eaab293106a85ba909be864/PowerEditor/src/ScitillaComponent/Buffer.cpp#L677
is not checking if the provided bufferID is a valid one. So I guess that needs to be done somewhere at the wrapper level.
Similar to https://github.com/notepad-plus-plus/notepad-plus-plus/blob/3a52ab1af16df8833eaab293106a85ba909be864/PowerEditor/src/ScitillaComponent/Buffer.cpp#L506
|
@ClaudiaFrank I merged your changes as is and will work on the remaining issues observed. |
| @@ -35,7 +38,7 @@ void export_notepad() | |||
| .def("destroyScintilla", &NotepadPlusWrapper::destroyScintilla, boost::python::args("editor"), "Destroy a Scintilla handle created with createScintilla.") | |||
| .def("getCurrentDocIndex", &NotepadPlusWrapper::getCurrentDocIndex, boost::python::args("view"), "Gets the current active index for the given view (0 or 1)") | |||
| .def("setStatusBar", &NotepadPlusWrapper::setStatusBar, boost::python::args("section", "text"), "Sets the status bar text. Call with the STATUSBARSECTION, and the string to show") | |||
| .def("getPluginMenuHandle", &NotepadPlusWrapper::getPluginMenuHandle, "Gets the handle for the Plugins menu.") | |||
| .def("getPluginMenuHandle", &NotepadPlusWrapper::getPluginMenuHandle, "DEPRECATED Gets the handle for the Plugins menu.") | |||
There was a problem hiding this comment.
@ClaudiaFrank What is the replacement? getMenuHandle?
There was a problem hiding this comment.
Correct, getMenuHandle, currently, accepts an integer value of 0 and 1.
Using 0 returns the plugins menu handle whereas if 1 is provided returns main menu handle.
- added checkForValidBuffer to test for valid/existing bufferIDs before calling N++. N++ crashes on invalid bufferIDs. - removed training whitespaces - added modification on unused NotepadPlusBuffer.h to support 64bit values
| // No additional parameters | ||
| break; | ||
|
|
||
| case NPPN_READONLYCHANGED: | ||
| params["bufferID"] = notifyCode->nmhdr.hwndFrom; | ||
| params["bufferID"] = (unsigned int)notifyCode->nmhdr.hwndFrom; |
There was a problem hiding this comment.
Why is this cast be needed? Currently I think it is not necessary.
| @@ -118,11 +127,15 @@ void NotepadPlusWrapper::notify(SCNotification *notifyCode) | |||
| } | |||
| break; | |||
|
|
|||
|
|
|||
| case NPPN_DOCORDERCHANGED: | |||
| params["newIndex"] = (unsigned int)notifyCode->nmhdr.hwndFrom; | |||
There was a problem hiding this comment.
Why is this cast be needed? Currently I think it is not necessary.
| default: | ||
| // Unknown notification, so just fill in the parameters as integers. | ||
| params["idFrom"] = notifyCode->nmhdr.idFrom; | ||
| params["hwndFrom"] = notifyCode->nmhdr.hwndFrom; | ||
| params["hwndFrom"] = (unsigned int)notifyCode->nmhdr.hwndFrom; |
There was a problem hiding this comment.
Why is this cast be needed? Currently I think it is not necessary.
There was a problem hiding this comment.
On x64 and without uint cast I observed crashes.
ghost
deleted the
2nd attempt - added missing exception classes