• Bugs in GoldEd

    From Nil Alexandrov@2:5015/46 to All on Sun May 9 06:07:18 2021
    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
    --- GoldED+/LNX 1.1.5
    * Origin: -=NIL BBS=- (2:5015/46)
  • From Eugene Palenock@2:5020/2065 to Nil Alexandrov on Sun May 9 11:50:52 2021
    Привет, Nil!

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

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

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

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

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

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


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

    ---
    * Origin: ... (2:5020/2065)
  • From Semen Panevin@2:5025/121 to Nil Alexandrov on Sun May 9 17:26:18 2021
    Доброго здоровьица тебе, Nil!

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

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

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

    ... Век живи, век лечись...
    --- GoldED+/LNX 1.1.5-b20180707 (Linux 5.4.80-gentoo-r1 iF6M10)
    * Origin: IceLAN (2:5025/121)
  • From Alexander Polozov@2:5025/38.89 to Semen Panevin on Sun May 9 23:03:52 2021
    Привет, Semen!


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

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

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

    ... Шли к рынку, а пришли к базару.
    --- GoldED+/LNX 1.1.5 (Linux 5.10.27-gentoo CPU UNKNOWN)
    * Origin: Если лошадь вам говорит, что вы псих, то это так и ес (2:5025/38.89)
  • From Mykhailo Kapitanov@2:467/239 to Nil Alexandrov on Sat May 22 21:38:08 2021

    Hello Nil!

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

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

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

    Mykhailo


    --- GoldED+/W64-MSVC 1.1.5-b20180707
    * Origin: Per rectum ad astra (2:467/239)