Macro Browser: Full management (Add, Edit, Delete, Enable/Disabled)

[akruphi]

Просьба к пользователям и знатокам макросистемы far2 (@shmuz @bigvax @michel-nk и всем остальным) высказаться не забыта ли какая особенность макросов, а также насколько удобна и корректна работа.

@elfmz вроде всё рабочее и можно мержить. Однако совсем навороченные макросы я не гоняю, поэтому полноценную многоязычность и помощь сделаю позже - вдруг укажут на недостатки и придется корректировать диалоги/сообщения.

[michaellukashov]

---

Off-by-one bug: imacro > 0 excludes macro #0 from no-change optimization

Location: macro.cpp — KeyMacro::MacroReplaceAdd()

	if (imacro > 0        // BUG: should be >= 0
			&& MacroLIB[imacro].Key == mr.Key
			&& ...
			&& !memcmp(...) ) {
		free(mr.Buffer);
		return -7; // no changes
	}

Impact: Editing macro at index 0 will always delete and recreate it, even if the user made zero changes. This wastes I/O (if AutoSaveSetup is on), breaks MFLAGS_NEEDSAVEMACRO bookkeeping for unrelated macros, and could perturb IndexMode bookkeeping unnecessarily.

Fix: if (imacro >= 0 && ...)

---

IsEditChanged() crashes on macros with nullptr Description or Src

Location: macrobrowser.cpp — IsEditChanged() + all callers in MacroEditDlgProc

inline static bool IsEditChanged(HANDLE hDlg, int EditControl, const wchar_t *Original)
{
	return 0 != StrCmp(
		reinterpret_cast<LPCWSTR>(SendDlgMessage(hDlg, DM_GETCONSTTEXTPTR, ...)),
		Original);   // 💥 nullptr deref if Original == nullptr
}

Impact: StrCmp is wcscmp-like. Macros loaded without a Description (or deleted macros where Description was nulled) have Description == nullptr. If the user opens such a macro in the edit dialog, the dialog proc crashes on DN_INITDIALOG or DN_EDITCHANGE.

Evidence: MacroDelete() explicitly sets Description = nullptr. The browser marks deleted items LIF_GRAYED, but if VMenu still allows F4/Enter on grayed items, this is an immediate crash. Even for normal macros, the registry loader may leave Description == nullptr when the key is absent.

Fix: Guard all IsEditChanged calls with null-safe comparison:

inline static bool IsEditChanged(HANDLE hDlg, int EditControl, const wchar_t *Original)
{
	const wchar_t *Cur = reinterpret_cast<LPCWSTR>(SendDlgMessage(hDlg, DM_GETCONSTTEXTPTR, EditControl, 0));
	if (!Cur && !Original) return false;
	if (!Cur || !Original) return true;
	return 0 != StrCmp(Cur, Original);
}

Also guard MacroEditDlg_SetDefault where it sends DM_SETTEXTPTR with potentially-null Src/Description.

---

Editing a disabled macro silently re-enables it

Location: macrobrowser.cpp — MacroEditDlgProc DN_CLOSE handler

The edit dialog computes Flags from scratch based only on its own checkboxes. There is no checkbox for MFLAGS_DISABLEMACRO, and the original macro's disabled state is not OR'd into the new flags.

Impact: User opens a disabled macro (-), presses F4, changes nothing, presses OK → macro is now enabled (+). This is data loss / unexpected state mutation.

Fix: In DN_CLOSE, before calling MacroReplaceAdd, preserve the original disabled flag when editing:

if (MEParam->imacro >= 0 && (MEParam->mr->Flags & MFLAGS_DISABLEMACRO))
    Flags |= MFLAGS_DISABLEMACRO;

---

Use-after-free when editing/deleting a currently executing macro

Location: macro.cpp — MacroDelete() / MacroReplaceAdd()

bool KeyMacro::MacroDelete(int imacro, bool bfull)
{
	...
	free(MacroLIB[imacro].Buffer);   // 💥 freed while macro engine may be executing it

MacroBrowser::Show() guards with !Macro->IsRecording() but never checks IsExecuting(). The macro engine's Work.MacroWORK and Work.ExecLIBPos may be pointing into MacroLIB[imacro].Buffer. Freeing it is classic UAF.

Impact: Editing or deleting the macro that is currently running crashes far2l.

Assessment: The underlying macro system lacks reference-counting or execution leases on MacroRecord buffers. This is a pre-existing architectural flaw, but PR #3416 makes it trivially easy to trigger because the browser now exposes Edit/Delete for any macro at any time. Previously, macro editing was tightly coupled to the assignment dialog during recording.

Recommendation: Before MacroDelete or MacroReplaceAdd mutates a macro, verify Work.ExecLIBPos is not targeting that record. At minimum, the browser should refuse to edit/delete while IsExecuting().

---

Hardcoded English strings (author acknowledges, but must block merge)

Location: macrobrowser.cpp — multiple Message() calls and dialog titles

The PR adds ~15 hardcoded English UI strings (e.g., "Reload all macros and lose unsaved ones?", "Mark macro as deleted?", "Empty keyboard shortcut field"). The author explicitly says: "полноценную многоязычность и помощь сделаю позже".

Impact: Breaks far2l's existing Russian (and other future) localization. Merging this creates technical debt that often never gets paid.

Recommendation: Do not merge until strings are wired through Msg:: or a new .lng block. The PR already touches lng.h-related infrastructure (adding Msg::MacroEdit* would be trivial in comparison to the C++ changes).

---

MacroDelete does not physically remove the record; deleted records accumulate in RAM

Deleted macros become zombie entries:

MacroLIB[imacro].Buffer = nullptr;
MacroLIB[imacro].BufferSize = 0;
MacroLIB[imacro].Src = nullptr;
MacroLIB[imacro].Description = nullptr;
MacroLIB[imacro].Flags |= MFLAGS_NEEDSAVEMACRO;

These zombies are skipped by GetIndex (good) but still iterated in SaveMacros, Sort, and PrepareVMenu. In a long-running far2l session with heavy macro editing, MacroLIBCount grows monotonically.

Impact: Memory bloat; Sort() becomes O(n log n) on ever-growing n including dead weight.

Fix: Compact MacroLIB during Sort() or provide a PurgeDeleted() pass. Alternatively, MacroDelete should memmove the tail down immediately (since Sort() is called right after anyway).

---

Reset button uses wrong message ID

Location: macrobrowser.cpp — dialog template

{DI_BUTTON, ..., DIF_CENTERGROUP | DIF_BTNNOCLOSE, Msg::ConfigDateTimeCurrent }

Msg::ConfigDateTimeCurrent is "Current" (as in "current date/time"). It is being reused for a Reset button labeled [ Сброс к текущему ]. In English this renders as a confusing "Current" button.

Fix: Add a proper Msg::MacroEditReset (or reuse an existing "Reset" message).

[akruphi]

@michaellukashov огромное спасибо за натравливание разумной нейронки на код.

Все основные замечания поправил.

1. Off-by-one bug: imacro > 0 excludes macro #0 from no-change optimization

Поправлено: как предложено

2. IsEditChanged() crashes on macros with nullptr Description or Src

Поправлено: как предложено

3. Editing a disabled macro silently re-enables it

Поправлено: в диалог редактирования добавлена галочка "[ ] Macro Active"

4. Use-after-free when editing/deleting a currently executing macro

Поправлено: перед всеми изменяющими действиями сейчас проверка (!Macro->IsExecuting() || !Macro->IsRecording())

5. Hardcoded English strings (author acknowledges, but must block merge)

Многоязыковость сейчас специально не доделана - немного позже, когда станет ясно,
что всё работает и по отзывам текущий интерфейс удобен и не требует переделок.

6. MacroDelete does not physically remove the record; deleted records accumulate in RAM

В этом PR в древнем коде я специально ничего не менял.
Его refactoring требует отдельного подхода.
Пока приспосабливаемся к существующей логике без её изменений.

7. Reset button uses wrong message ID

О! Точно. Есть же Msg::Reset

[shmuz]

Поправлено: перед всеми изменяющими действиями сейчас проверка
(!Macro->IsExecuting() || !Macro->IsRecording())

Видимо здесь должно быть &&, а не ||.

[akruphi]

Поправлено: перед всеми изменяющими действиями сейчас проверка
(!Macro->IsExecuting() || !Macro->IsRecording())

Видимо здесь должно быть &&, а не ||.

Точно! Большое спасибо. Поправил.

[michaellukashov]

1. BLOCKER: free on value-pointer when BufferSize == 1 — crash on duplicate / error paths

File: far2l/src/macro/macro.cpp (new MacroReplaceAdd)
Evidence: __parseMacroString normalizes single-opcode buffers by storing the value in the pointer and freeing the heap block. MacroReplaceAdd calls
free(mr.Buffer) in the -7 (identical), -8 (duplicate), -9 (realloc fail), and -10 (delete fail) paths without checking mr.BufferSize > 1.
Why it matters: Every macro that compiles to a single opcode (e.g., simple key sequences, trivial calls) will crash the editor if the user hits a
duplicate key conflict or cancels after making no effective changes.
Suggested change: Replace all free(mr.Buffer) calls in MacroReplaceAdd with a conditional:

  if (mr.BufferSize > 1)
      free(mr.Buffer);

Or, better, extract a small helper that mirrors the ownership semantics used in __parseMacroString.

2. MAJOR: Dialog layout overlap — active-panel "Selection" checkbox placed on wrong row

File: far2l/src/macro/macrobrowser.cpp
Evidence: The DialogDataEx array defines:

• ME_TEXT_A_FOLDERS_CHMARK at (6, 15)
• ME_CHECKBOX_A_FOLDERS at (7, 15)
• ME_TEXT_A_SELECTION_CHMARK at (6, 15) ← same row as folders
• ME_CHECKBOX_A_SELECTION at (7, 15) with Y2 = 16 ← wrong row, Y2 inconsistent

The ASCII art comment in the source shows row 16 for selection, but the code places it on row 15. This causes the selection checkbox to overlap the
folders row in the active-panel column.
Why it matters: The dialog renders with overlapping controls; on TTY backends this may corrupt the layout or make the selection checkbox unclickable.
Suggested change: Move ME_TEXT_A_SELECTION_CHMARK and ME_CHECKBOX_A_SELECTION to Y = 16, matching the ASCII layout and the passive-panel side.

3. MAJOR: Null dereference on pstrKeyName in DN_CLOSE

File: far2l/src/macro/macrobrowser.cpp (new MacroEditDlgProc)
Evidence:

  const wchar_t *pstrKeyName = (const wchar_t*)SendDlgMessage(hDlg, DM_GETCONSTTEXTPTR, ME_EDIT_KEY, 0);
  if (*pstrKeyName == 0 || *(pstrKeyName + wcsspn(pstrKeyName, L" \t\n\r\f\v")) == 0) {

DM_GETCONSTTEXTPTR can return nullptr in some dialog states (empty edit control, control not yet initialized). Dereferencing without a null check is
unsafe.
Suggested change:

  if (!pstrKeyName || *pstrKeyName == 0 || ...)

File: far2l/src/macro/macro.hpp, macro.cpp
Evidence: MacroBrowser::Show wraps the calls with !Macro->IsExecuting() && !Macro->IsRecording(). But MacroReplaceAdd and MacroDelete are now public
methods on KeyMacro. If any future caller invokes them while a macro is running, Sort() can reorder MacroLIB and invalidate Work.MacroWORK, leading to a
use-after-free or wild opcode execution.
Why it matters: New public APIs that mutate the macro array during execution are a latent time bomb for anyone wiring them into plugins or key handlers.
Suggested change: Add an early return or ASSERT inside MacroReplaceAdd and MacroDelete checking !Work.Executing && !Recording. The browser can then drop
its redundant guards or keep them as defense-in-depth.

5. MINOR: Duplicate check misses disabled macros

File: far2l/src/macro/macro.cpp (MacroReplaceAdd)
Evidence: Duplicate detection uses GetIndex(mr.Key, iarea, false). GetIndex skips disabled macros (!(MPtr->Flags & MFLAGS_DISABLEMACRO)). A disabled macro
and a newly added macro can therefore share the same key in the same area. Enabling the old one later creates ambiguity.
Why it matters: This is pre-existing behavior inherited from the recording path (ProcessKey uses the same GetIndex call), so it is not a new regression
introduced by this PR, but the PR extends the surface area where users can hit it.

[akruphi]

1. BLOCKER: free on value-pointer when BufferSize == 1 — crash on duplicate / error paths

Поправлено: как предложено - добавил везде проверку (mr.BufferSize > 1).

2. MAJOR: Dialog layout overlap — active-panel "Selection" checkbox placed on wrong row

Поправлено: как предложено

3. MAJOR: Null dereference on pstrKeyName in DN_CLOSE

Поправлено: как предложено

4. Suggested change: Add an early return or ASSERT inside MacroReplaceAdd and MacroDelete checking !Work.Executing && !Recording.

Разумно. Поставил проверку if (IsExecuting() || IsRecording()) в самом начале у KeyMacro::MacroDelete() и KeyMacro::MacroReplaceAdd()

5. MINOR: Duplicate check misses disabled macros

Поправлено: проще оказалось отказаться от использования GetIndex() и самому в цикле перебрать соответствующие макросы

---

@michaellukashov Спасибо за замечания. 1, 4 и 5 реально баги, вылезшие из-за недостаточного моего внимания и вникания в логику текущего древнего кода.

Просьба приносить ещё, если что заметится.

[michaellukashov]

Critical: MacroReplaceAdd fails when adding a new macro that reuses a key from a previously deleted macro.

• Location: far2l/src/macro/macro.cpp:6895-6901
• Problem: When imacro < 0 (adding a new macro) and a deleted macro with the same key exists in the target area, the code enters this branch:

    if (!MacroLIB[ipos].BufferSize || !MacroLIB[ipos].Src) {
        if (!MacroDelete(imacro, false)) {  // imacro == -1 here!
            ...
            return -11; // Delete error
        }
        imacro = ipos;
    }

  MacroDelete(-1, false) immediately returns false because imacro < 0 fails the bounds check. This causes the entire add operation to abort with error -11
  ("Internal error"), even though the intent was simply to reuse the deleted slot.
• Fix: Skip MacroDelete when imacro < 0 (there is no existing macro to delete before reusing the deleted slot):

    if (imacro >= 0) {
        if (!MacroDelete(imacro, false)) {
            if (mr.BufferSize > 1)
                free(mr.Buffer);
            return -11;
        }
    }
    imacro = ipos;

Important: Duplicate-key loop finds the last match instead of the first.

• Location: far2l/src/macro/macro.cpp:6886-6894
• Problem: The loop that checks for duplicate keys does not break after finding a match. ipos is overwritten by every subsequent match. This is copied
  from GetIndex() behavior, but if there are multiple deleted entries for the same key, it will always select the last one. If the intent is deterministic
  first-match behavior, add break; inside the if.

FYI: View() prints mr->Description via %ls without null-check.

• Location: far2l/src/macro/macrobrowser.cpp:632
• Problem: Deleted macros can have Description == nullptr (set by MacroDelete). View() allows viewing grayed/deleted entries via F3. Passing nullptr to
  %ls is UB. Pre-existing, but now more visible because deleted macros are explicitly shown as grayed.

[akruphi]

1. Critical: MacroReplaceAdd fails when adding a new macro that reuses a key from a previously deleted macro.

Поправлено: как предложено

2. Important: Duplicate-key loop finds the last match instead of the first.

Поправлено: как предложено

3. FYI: View() prints mr->Description via %ls without null-check.

При выводе через %ls строк mr->Description и mr->Src в нескольких местах добавил проверки. Нейронке, конечно спасибо, но она только одно место усмотрела.

[spnethw]

Удобное! Замеченные баги:

1. Кнопка << Assign Key вызывает диалог, в котором задаётся не только хоткей, но и описание макроса. Описание не подтягивается из существующего, и нужно вручную вбивать его повторно в этом диалоге, иначе оно сбросится.
2. Этот же диалог: при нажатии Esc отменяется корректно, а при закрытии мышкой с кнопки "Отмена" в MacroBrowser существующие описание и хоткей сбрасываются.
3. При отключении макроса он почему-то дублируется в файле:

[KeyMacros/Shell/AltNumEnter]
Description=OpenWith
DisableOutput=0x1
EmptyCommandLine=0x1
NoEVSelection=0x1
Sequence=F11 $if(menu.select("OpenWith",0)>0) Enter $else Esc $end

[KeyMacros/Shell/~AltNumEnter]
Description=OpenWith
DisableOutput=0x1
EmptyCommandLine=0x1
NoEVSelection=0x1
Sequence=F11 $if(menu.select("OpenWith",0)>0) Enter $else Esc $end

[akruphi]

@spnethw все перечисленные баги исходят из текущей логики древнего кода - поправим, но не сейчас.

В текущем PR я специально никак не лезу в существующий код, чтобы случайно не испортить - уж больно там закручено и я не уверен, что всех заложенных древними драконов и вывертов осознал.

Текущий PR - это пока продвинутый прототип, где главное понять, что взаимодействие с макроподситемой во всех случаях корректно и ничего не ломает.

1 и 2 - это надо в KeyMacro::AssignMacroKey( править - оказывается оно не всегда возвращает KEY_INVALID при отказе от действия.

3 - связано с логикой KeyMacro::SaveMacros(, которая переписывает только имеющиеся в MacroLIB макросы, не трогая остальное, а у отключенных исторически имя меняется - они начинаются с ~.

[akruphi]

Добавлено в редактирование изменение флага макросов MFLAGS_NOSENDKEYSTOPLUGINS.

Заменено в диалоге редактирования для Последовательности
поле с DI_EDIT на DI_MEMOEDIT с подсветкой через FarColorer синтаксиса макросов.

По совету @exkrexpexfex в dialog.cpp для DI_MEMOEDIT добавлена поддержка в DM_GETCONSTTEXTPTR.
Просьба @exkrexpexfex проверить корректность и нет ли тут утечек памяти.

Выяснилось, что сейчас у DI_MEMOEDIT не обрабатываются полноценно
ни DN_EDITCHANGE, ни DM_LISTGETCURPOS, ни DM_GETSELECTION
- в результате у поля не ловится изменение и вставка только в начало строки
- оставим это пока так, ибо и так для одного PR много чего наворочено.

@michaellukashov просьба по возможности текущий вариант также прогнать через Вашу AI - вдруг что ещё нейронка посоветует дельного.

[michaellukashov]

#1: StrCmp NULL dereference in MacroReplaceAdd "no change" optimization

Severity: BLOCKER — guaranteed crash on common input

Location

far2l/src/macro/macro.cpp — KeyMacro::MacroReplaceAdd()

Code

if (imacro >= 0
        && MacroLIB[imacro].Key == mr.Key
        && (int)(MacroLIB[imacro].Flags & MFLAGS_MODEMASK) == iarea
        && (MacroLIB[imacro].Flags & ~MFLAGS_MODEMASK & ~MFLAGS_NEEDSAVEMACRO)
            == (Flags & ~MFLAGS_MODEMASK & ~MFLAGS_NEEDSAVEMACRO)
        && MacroLIB[imacro].BufferSize == mr.BufferSize
        && (mr.BufferSize > 1
            ? !memcmp(MacroLIB[imacro].Buffer, mr.Buffer, mr.BufferSize * sizeof(*mr.Buffer))
            : MacroLIB[imacro].Buffer == mr.Buffer)
        && !StrCmp(MacroLIB[imacro].Description, strDescription) ) {  // ← CRASH
    if (mr.BufferSize > 1)
        free(mr.Buffer);
    return -8;
}

Mechanism

MacroLIB[imacro].Description is nullptr in two common cases:

1. Macro loaded from INI without Description= key — LoadMacros only sets Description when the key is present. Macros created by recording (Ctrl-.) never have a description. Macros manually added in key_macros.ini may omit the description field.

2. Previously deleted macro — MacroDelete() explicitly sets Description = nullptr.

Meanwhile, strDescription is a FARString constructed from the dialog's description edit field. It is never nullptr — it is at minimum an empty string L"".

StrCmp(const wchar_t*, const wchar_t*) is defined in far2l/src/locale/locale.hpp:109-112:

inline int __cdecl StrCmp(const wchar_t *s1, const wchar_t *s2)
{
    return WINPORT(CompareString)(0, SORT_STRINGSORT, s1, -1, s2, -1) - 2;
}

The -1 length means "null-terminated, determine length by scanning." With s1 == nullptr, CompareString walks from address 0 → segfault.

Note: the StrCmpNN/StrCmpNNI variants use NORM_STOP_ON_NULL which would treat NULL as an empty string, but plain StrCmp does not — confirmed by comparison with arclite/src/locale.hpp:104-106 which has identical semantics.

Trigger

1. Open Macro Browser
2. Select any macro that has no description (recorded macros, hand-edited INI entries without Description=)
3. Press F4 / Enter to edit → dialog opens with empty description field
4. Change nothing → press OK
5. MacroReplaceAdd detects that nothing changed → evaluates !StrCmp(NULL, "") → crash

Why this is a blocker

• No corner case: most macros in the wild lack descriptions — recording macros via Ctrl-. is the primary way users create macros, and those macros never get a Description
• Silent data loss: the crash occurs AFTER the "no change" detection succeeds on all OTHER fields, so mr.Buffer is freed — but the macro is never actually saved since the process crashes
• The earlier IsEditChanged null guard (already fixed by @michaellukashov's review) doesn't help: that guard is in the dialog proc detecting whether the edit field changed. If the user doesn't modify the description field, the dialog still proceeds to MacroReplaceAdd → crash

Fix

// Replace:
        && !StrCmp(MacroLIB[imacro].Description, strDescription) ) {

// With:
        && ((!MacroLIB[imacro].Description && strDescription.IsEmpty())
            || (MacroLIB[imacro].Description
                && !StrCmp(MacroLIB[imacro].Description, strDescription))) ) {

---

#2: *mr.Buffer dereference through value-pointer when BufferSize == 1

Severity: HIGH — currently dead code, but a latent crash

Location

far2l/src/macro/macro.cpp — KeyMacro::MacroReplaceAdd(), Buffer assignment block

Code

// Set data to macro
MacroLIB[ipos].Key = mr.Key;

if (mr.BufferSize > 1)
    MacroLIB[ipos].Buffer = mr.Buffer;
else if (mr.Buffer && mr.BufferSize > 0)                              // ← BufferSize == 1
    MacroLIB[ipos].Buffer = reinterpret_cast<DWORD *>((DWORD_PTR)(*mr.Buffer));  // ← CRASH
else if (!mr.BufferSize)
    MacroLIB[ipos].Buffer = nullptr;
MacroLIB[ipos].BufferSize = mr.BufferSize;

Mechanism

__parseMacroString (syntax.cpp:2021–2026) uses a value-in-pointer optimization for BufferSize == 1:

if (CurMacroBufferSize > 1)
    CurMacroBuffer = CurMacro_Buffer;                                    // keep heap pointer
else if (CurMacro_Buffer) {
    CurMacroBuffer = reinterpret_cast<DWORD *>((DWORD_PTR)(*CurMacro_Buffer)); // VALUE in pointer
    free(CurMacro_Buffer);                                               // free the heap buffer
}

When BufferSize == 1, mr.Buffer is not a heap pointer — it is the actual opcode DWORD value stored as a pointer address. For example, MCODE_OP_PLAINTEXT = 0x6000ABCD → mr.Buffer = (DWORD*)0x6000ABCD.

The line *mr.Buffer then dereferences address 0x6000ABCD, reading from unmapped memory → segfault.

The correct value-in-pointer copy is a direct assignment:

MacroLIB[ipos].Buffer = mr.Buffer;

This is exactly what SetOpCode() already does (macro.cpp:6704–6709):

} else {
    OldOpCode = (DWORD)(DWORD_PTR)MR->Buffer;
    MR->Buffer = (DWORD *)(DWORD_PTR)OpCode;   // direct assignment, no dereference
}

Is it currently reachable?

No — in the current codebase, ParseMacroString always normalizes single-opcode macros by appending MCODE_OP_NOP (syntax.cpp:1989–2002), so mr.BufferSize after a successful parse is always ≥ 2. The else if branch is dead code.

Why it must still be fixed

Despite being unreachable, this is a latent crash:

1. SetOpCode already handles BufferSize == 1 (macro.cpp:6704–6709). The runtime execution engine is designed to work with BufferSize == 1 records. MacroReplaceAdd creates records that the execution engine can process — it must handle all states the engine handles.

2. If the NOP-padding in __parseMacroString is ever changed or removed, every macro save through the browser crashes. The padding is an implementation detail of the parser, not a contract.

3. BufferSize == 1 can be synthesized through SetOpCode at runtime. If any code path constructs or mutates a macro record to BufferSize == 1 and then passes it through the edit flow, this crashes.

4. The fix is one line — no reason to leave the landmine.

Fix

// Replace:
    else if (mr.Buffer && mr.BufferSize > 0)
        MacroLIB[ipos].Buffer = reinterpret_cast<DWORD *>((DWORD_PTR)(*mr.Buffer));

// With:
    else if (mr.Buffer && mr.BufferSize > 0)
        MacroLIB[ipos].Buffer = mr.Buffer;

This mirrors __parseMacroString's own value-pointer assignment (syntax.cpp:2024) and SetOpCode's write path (macro.cpp:6709).

---

Additional Minor Observations

1. MacroEditDlg_SetDefault passes potentially-NULL Description and Src to DM_SETTEXTPTR

SendDlgMessage(hDlg, DM_SETTEXTPTR, ME_EDIT_DESCRIPTION,
    reinterpret_cast<LONG_PTR>(MEParam->mr->Description));
SendDlgMessage(hDlg, DM_SETTEXTPTR, ME_MEMOEDIT_SEQUENCE,
    reinterpret_cast<LONG_PTR>(MEParam->mr->Src));

For new macros (imacro < 0), mr points to mr0 which has strEmpty for both — safe. For editing an existing macro, both fields can be nullptr. The new DM_GETCONSTTEXTPTR handler for DI_MEMOEDIT (added in this PR to dialog.cpp) calls GetString() on a DlgEdit* — which may handle NULL input gracefully, but DI_EDIT (for Description) behavior with DM_SETTEXTPTR NULL should be verified.

2. LIF_SELECTED flag on all macro library items

In MacroLib_KeywordsFunctions2Items(), all keyword/function/code items are created with LIF_SELECTED:

Items.back().Flags = LIF_SELECTED;

This puts a checkmark on every item in the "MacroLibrary" listbox. Not a bug, but visually noisy for 100+ entries.

[akruphi]

Вроде уже удобно рабочее и возможные некорректности и утечки памяти выловлены. Большое спасибо @michaellukashov и @exkrexpexfex за советы и выверку кода.

Что осталось (но это точно уже за рамками этого PR и так набралось изменений выносящих мозг 🤯):

• аккуратно перевести все надписи и разместить их в farlang.templ.m4
• описать MacroBrowser в отдельных разделах помощи
• корректировка логики KeyMacro::SaveMacros() и KeyMacro::MkRegKeyName(), чтобы при изменении статуса MFLAGS_DISABLEMACRO меняло макрос, а не плодило новую запись - сейчас заблокированные макросы сохраняются с ~ перед именем клавиши и текущая реализация KeyMacro::SaveMacros() в результате не удаляет версию макроса с противоположным статусом
• для DI_MEMOEDIT не реализована работа DM_GETCURSORPOS (Y возвращает как надо, а X - некорректно), DM_GETSELECTION, DM_SETSELECTION и DN_EDITCHANGE вообще не работают (в коде far2l/src/dlgedit.cpp множественные заглушки для DI_MEMOEDIT)