Кресты / Говнокод #27785 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
  16. 16
  17. 17
  18. 18
  19. 19
  20. 20
  21. 21
  22. 22
  23. 23
  24. 24
  25. 25
  26. 26
  27. 27
  28. 28
  29. 29
  30. 30
  31. 31
  32. 32
  33. 33
  34. 34
  35. 35
  36. 36
  37. 37
  38. 38
  39. 39
  40. 40
  41. 41
  42. 42
  43. 43
  44. 44
  45. 45
  46. 46
  47. 47
  48. 48
  49. 49
  50. 50
  51. 51
  52. 52
  53. 53
  54. 54
  55. 55
  56. 56
  57. 57
  58. 58
  59. 59
template<typename T>
static ALWAYS_INLINE void FormatLogMessageAndPrintW(
                                             const char* channelName, 
                                             const char* functionName, 
                                             LOGLEVEL level,
                                             const char* message, 
                                             bool timestamp, 
                                             bool ansi_color_code,
                                             bool newline, 
                                             const T& callback)
{
  char buf[512];
  char* message_buf = buf;
  int message_len;
  if ((message_len = FormatLogMessageForDisplay(message_buf,
                       sizeof(buf), channelName, functionName,
                       level, 
                       message, timestamp,
                       ansi_color_code, newline)) > (sizeof(buf) - 1))
  {
    message_buf = static_cast<char*>(std::malloc(message_len + 1));
    message_len = FormatLogMessageForDisplay(message_buf, 
                 message_len + 1, channelName, functionName, 
                 level, message, timestamp, ansi_color_code, newline);
  }
  if (message_len <= 0)
    return;

  // Convert to UTF-16 first so unicode characters display correctly.
  // NT is going to do it anyway...
  wchar_t wbuf[512];
  wchar_t* wmessage_buf = wbuf;
  int wmessage_buflen = countof(wbuf) - 1;
  if (message_len >= countof(wbuf))
  {
    wmessage_buflen = message_len;
    wmessage_buf = static_cast<wchar_t*>
               (std::malloc((wmessage_buflen + 1) * sizeof(wchar_t)));
  }

  wmessage_buflen = MultiByteToWideChar(CP_UTF8, 0, message_buf,
                    message_len, wmessage_buf, wmessage_buflen);

  if (wmessage_buflen <= 0)
    return;

  wmessage_buf[wmessage_buflen] = '\0';
  callback(wmessage_buf, wmessage_buflen);

  if (wmessage_buf != wbuf)
  {
    std::free(wbuf);                        // <=
  }

  if (message_buf != buf)
  {
    std::free(message_buf);
  }
}

Отсюда:
https://pvs-studio.com/ru/blog/posts/cpp/0880/

Запостил: HO9I6PbCKuu_neTyx HO9I6PbCKuu_neTyx, (Updated )

Комментарии (31) RSS

  • Любопытно.

    Надо было использовать scoped_pointer или как его зовут?
    Ответить
    • Тут костыль на костыле.

      Автор решил, что если буфер укладывается в 512 байтиков, для него нецелесообразно дёргать дорогостоящий malloc, хватит и стека. А если не уложится, можно и malloc дёрнуть.

      И вся печаль в реализации: он решил в стеке предварительно выделить буфера по 512 байтиков, а позже, если не хватит, маллокать и переставлять указатель. Багор возникает при освобождении памяти: нужно узнать, какой буфер использовался: из стека или из маллока. И вот тут в одну из веток прокралась опечатка.
      Ответить
      • А я в одном из логгеров тупо обрезаю на 512... Ибо память выделять нечем.
        Ответить
      • auto_ptr<wchar_t> wmessage_buf(wbuf);
        ...
        if (message_len >= countof(wbuf))
        {
          wmessage_buflen = message_len;
          wmessage_buf.reset(...malloc...);
        }
        ...
          if (wmessage_buf.get() != wbuf)
          {
            (void)wmessage_buf.release();
          }

        Всё! И анализатор не ругнется, и память освободится автоматически!
        Ответить
    • Тут можно либо unique_ptr с кастомным делитером, либо просто зафигачить scope_exit с одной из двух функций.
      Ответить
      • А в бусте случаем нету готового класса, который N байтов на стеке, а потом реаллочится в кучу?

        З.Ы. Есть, boost::small_vector<uint8_t, 512>
        Ответить
  • Ох ебать, кто-то попробовал сделать free() для выделенного на стеке массива, анализатор это словил, а потом кто-то подумал, что это якобы баг анализатора, ну ничего ж себе!!! Даже какую-то картинку для этого нарисовали!
    Это правда заслуживает отдельной статьи и каких-то сложных рассуждений?
    Ответить
    • Мораль: неча на зеркало пенять, лучше проверь еще раз кривизну своей рожи.
      Ответить
    • Именно поэтому я за ``govnokod'': после первого готсе Андрей Карпов (или как там его) пивас здесь форсить перестал.
      Ответить
      • Давайте флудить и форсить.

        (чтобы по запросу "PVS" первой ссылкой выдавалось говнокод и goatse.)
        Ответить
        • http://pvs.csl.sri.com/

          > PVS is a mechanized environment for formal specification and verification. PVS consists of a specification language, a large number of predefined theories, a type checker, an interactive theorem prover that supports the use of several decision procedures and a symbolic model checker, various utilities including a code generator and a random tester, documentation, formalized libraries, and examples that illustrate different methods of using the system in several application areas. By exploiting the synergy between a highly expressive specification language and powerful automated deduction, PVS serves as a productive environment for constructing and maintaining large formalizations and proofs. See the description page for a summary of the features of PVS, and the documentation page for full details.

          Оно еще на LISP написано, не то что Coq
          Ответить
          • [size=9]Давайте флудить и форсить[/size]
            *================================================*
            |goat sex goat sex goat sex goat sex goat sex goa|
            | *============================================*t|
            |t|                  PVS-Studio                | | 
            |a|    Универсальный статический анализатор    |s|
            |o*============================================*e|
            |g xes toag xes toag xes toag xes toag xes toag x|
            *================================================*
            Ответить
      • Хули набросились на пивас? Сам им пользовался, хорошая штука: например, я случайно сделал free на выделенном на стеке массиве, и анализатор это словил.
        Ответить
  • Ненавижу этот паттерн... Практика показывает, что никто его с первого раза правильно не пишет.

    Ну почему не сделать отдельный указатель под выделенный буфер и отдельный указатель под активный буфер? И потом тупо работаем с active (в котором или стековый или allocated) и в конце free(allocated) без всяких там условий...

    Нет блядь, будем экономить на спичках...
    Ответить

Добавить комментарий для bormand Отменить ответ

Помни, guest, за тобой могут следить!

    А не использовать ли нам bbcode?


    8