Bugs in GoldEd

[Nil Alexandrov]

Hello, All!

Правильно я понимаю, что последняя версия golded лежит
https://github.com/golded-plus/golded-plus ?

Далее следуют мои наблюдения.

1. char buf[256], options[80];
Много функций имеют такие вот фиксированные буфера на стеке для работы с 80x25
терминалом, если запускать в Linux/Mac широком терминале, то происходит
переполнение.
Быстрое лечение - замена на бОльшие значения буферов.
Правильное лечение - узнать размер терминала, выделить буфер соответственно,
ещё надо ловить сигналы, когда размер терминала меняется и перевыделять буфера
и перерисовывать экран по-хорошему - это много кода и он платформенно
зависимый.

2. Overflow
Прочие переполнения фиксированных буферов, например, когда идёт *цветовое
выделение фрагмента*, можно предположить худший сценарий, когда весь текст
будет выделен.
-+- a/golded3/gectnr.cpp
+++ b/golded3/gectnr.cpp
@@ -95,7 +95,7 @@ void Container::StyleCodeHighlight(const char* text, int row,
int col, bool dohi

uint sclen = 0;
const char* txptr = text;
- char buf[200];
+ char* buf = (char*)throw_calloc(1, strlen(text)+1);
const char* ptr = text;
const char* stylemargins = " -|\\"; // we probably have to make a
keyword for it
char* punctchars = CFG->stylecodepunct;

3. New delete type mismatch
Есть места, где буфер выделают под массив, а освобождают как переменную,
например
-+- a/golded3/gearea.cpp
+++ b/golded3/gearea.cpp
@@ -151,8 +151,8 @@ void GPickArealist::do_delayed()
LOG.printf("! Buffer overflow: tmp in GPickArealist::do_delayed(), 15
or 16 lines above");
PointerErrorExit();
}
- delete buf;
- delete tmp;
+ delete[] buf;
+ delete[] tmp;
}


-+- a/golded3/geinit.cpp
+++ b/golded3/geinit.cpp
@@ -536,7 +536,7 @@ void Uninitialize()
{
if (WideUsername)
{
- delete WideUsername;
+ delete[] WideUsername;
WideUsername = NULL;
}
LookupNodeClear();

4. Strcpy param overlap
Есть код, где нужно удалить часть строки, например начальные Re: - это
реализуется путём копирования подстроки. Во многих местах для этого
используется strcpy(), тогда как для перекрывающихся участков памяти нужно
использовать memmove.
-+- a/golded3/gepost.cpp
+++ b/golded3/gepost.cpp
@@ -1076,7 +1076,7 @@ void MakeMsg(int mode, GMsg* omsg, bool ignore_replyto)
ptr = strskip_wht(r);
}
while(r);
- strcpy(msg->re, ptr);
+ memmove(msg->re, ptr, strlen(ptr)+1);
}
strcpy(msg->ddom, omsg->odom);
if((mode == MODE_REPLYCOMMENT) and omsg->fwdorig.net)
-+- a/golded3/getpls.cpp
+++ b/golded3/getpls.cpp
@@ -350,17 +350,17 @@ int TemplateToText(int mode, GMsg* msg, GMsg* oldmsg,
const char* tpl, int origa

// build @dpseudo
if(msg->to_me())
- strcpy(oldmsg->pseudoto, msg->pseudofrom);
+ memmove(oldmsg->pseudoto, msg->pseudofrom, strlen(msg->pseudofrom)+1);
else if(msg->to_you())
- strcpy(oldmsg->pseudoto, msg->pseudoto);
+ memmove(oldmsg->pseudoto, msg->pseudoto, strlen(msg->pseudoto)+1);
else
*(oldmsg->pseudoto) = NUL;

// build @opseudo
if(msg->by_me())
- strcpy(oldmsg->pseudofrom, msg->pseudofrom);
+ memmove(oldmsg->pseudofrom, msg->pseudofrom,
strlen(msg->pseudofrom)+1);
else if(msg->by_you())
- strcpy(oldmsg->pseudofrom, msg->pseudoto);
+ memmove(oldmsg->pseudofrom, msg->pseudoto, strlen(msg->pseudoto)+1);
else
*(oldmsg->pseudofrom) = NUL;

5. Memset() clearing an object with no trivial copy-assignment.
С многими C++ объектами работают, как если бы это были C-структуры с POD
типами, например, инициализация объекта сообщений происходит так memset(msg, 0,
sizeof(GMsg)), хотя это наследуемый класс от gmsg, и там есть много сложных
объектов типа ftn_addr, или даже gmsg_links, внутри которого есть std::vector.
Также, с целью оптимизации выделения/освобождения памяти, переиспользуют такие
объекты, просто забивая их нулями. Благодаря счастливому стечению
обстоятельств, нули работают как и для char*, и для ftn_addr и даже, скорее
всего, для локальной реализации std::vector, который хранит указатель на буфер
и размер. Что точно ломается - это память, которую выделяет внутри std::vector.
По хорошему, надо сделать рефакторинг, чтобы инициализировать объекты C++
соответствующими для этого методами.
Быстрый хак, чтобы не утекала память под std::vector, работает только начиная с
C++11.
-+- a/goldlib/gmb3/gmo_msg.h
+++ b/goldlib/gmb3/gmo_msg.h
@@ -250,6 +250,7 @@ public:
{
reply_to = reply_first = reply_next = 0;
reply_list.clear();
+ reply_list.shrink_to_fit();
}

int list_max() const

6. Memory leaks
Наверное их много, сходу ловиться вот такой.
-+- a/goldlib/gall/gespell.cpp
+++ b/goldlib/gall/gespell.cpp
@@ -834,6 +834,9 @@ void CSpellChecker::Close()
{
if (!IsInited()) return;
UnLoad();
+ CSpellLangV::iterator it;
+ for (it = mLangs.begin(); it != mLangs.end(); it++)
+ delete *it;
mLangs.clear();
mInited = false;
}

7. ISO C++ forbids converting a string constant to char*
Все строковые константы из .text сегмента передаются дальше по коду без
константы, что провоцирует много предупреждений со стороны компилятора. Хочется
надеяться, что ни какой код не начёт переписать содержимое таких буферов.

Best Regards, Nil

[Eugene Palenock]

Привет, Nil!

09 май 21 06:07, Nil Alexandrov -> All:

NA> Далее следуют мои наблюдения.

Напомню ещё один баг - повреждение текста при стилях выделения в квотинге.

Без кавычек эти три строки GoldEd не сохраняет:
"ZZZZ<YYYY"
"X)>AAA"
"X)>BBB"

Они превращаются в такое при сохранении:
"ZZZZ<YYYY"
"X)>AAA BBB"

Коллизия случается при совокупности условий:
- подряд три этих строки где-либо в текcте;
- в 1 строке есть стрелка влево в любом месте строки;
- в начале 2 и 3 строки нечто похожее на квотинг;
- причём 2 буква должна быть не буквой а каким-нибудь символом (если там "XX>"
то коллизии нет, а например "X=>" или "X(>" вызывает коллизию).


--
С уважением, Евгений.

[Semen Panevin]

Доброго здоровьица тебе, Nil!

Sunday May 09 2021 06:07, Nil Alexandrov писал All:

NA> Далее следуют мои наблюдения.
Если ты ещё найдёшь отчего у меня голдед при включенной проверке орфографии
крашится, а у других не крашится (и у меня раньше не крашился, а что изменилось
- хрен его знает) - будет вообще супер...

С наилучшими пожеланиями, Семён.

... Век живи, век лечись...

[Alexander Polozov]

Привет, Semen!


Ответ на сообщение Semen Panevin (2:5025/121) к Nil Alexandrov, написанное
09 май 21 в 17:26:

NA>> Далее следуют мои наблюдения.
SP> Если ты ещё найдёшь отчего у меня голдед при включенной проверке
SP> орфографии крашится, а у других не крашится (и у меня раньше не
SP> крашился, а что изменилось - хрен его знает) - будет вообще супер...
Ну найти это вряд-ли, глюк старый, известный и до сих пор ни кем не
отловленный...
Зато довольно давно известно лечение, сохраняешь старый конфиг в бекап,
создаёшь новый пустой конфиг, в который потом по одной строке переносишь только
необходимое (у меня на выходе новый конфиг уложился в 96 строк, включая пустые
для красоты), на малом количестве строк в конфиге глюк со спелчеком не вылазит.

Со всеми пожеланиями, Alexander Polozov

... Шли к рынку, а пришли к базару.

[Mykhailo Kapitanov]

Hello Nil!

09 May 21 06:07, you wrote to all:

NA> 1. char buf[256], options[80];
NA> Много функций имеют такие вот фиксированные буфера на стеке для работы
NA> с 80x25 терминалом, если запускать в Linux/Mac широком терминале, то

В версии для Windows та же проблема.

Mykhailo