PVS-Studio and CWE-401
Предлагаем вашему вниманию цикл статей, посвященных рекомендациям по написанию качественного кода на примере ошибок, найденных в проекте Chromium. Это третья часть, которая будет посвящена утечкам памяти.

Я считаю, что код проекта Chromium и используемые в нём библиотеки очень высокого качества. Да, в вводной статье я писал про 250 ошибок, но на самом деле — это очень маленькое число. В силу законов вероятности в огромном проекте обязательно найдётся немало ошибок.

Тем не менее, если говорить об утечках памяти, их не так уж и мало. Думаю, разработчиков Chromium подводит то, что они отдают предпочтение динамическим анализаторам кода. Конечно, у этих инструментов есть ряд преимуществ. Например, они не дают ложных срабатываний, ведь раз динамический анализатор обнаружил ошибку, значит она действительно есть.

С другой стороны, у динамического анализа есть и слабости. Если какой-то код не выполнился, то и ошибка обнаружена не будет. А любой программист понимает, что крайне сложно покрыть тестами 100% кода, вернее, на практике это просто невозможно. В результате, ряд ошибок остаётся в коде и ждёт благоприятного стечения обстоятельств, чтобы проявить себя.

Здесь на помощь может прийти статический анализатор кода. Да, это намёк разработчикам из компании Google, что мы будем рады, если они станут нашими клиентами. Более того, мы готовы на дополнительные работы по адаптации и настройке PVS-Studio под особенности проекта Chromium. Наша команда также готова взять на себя правку найденных ошибок. У нас уже был подобный опыт (пример).

Но вернёмся к утечкам памяти. Как вы увидите, они прячутся в коде, который крайне редко получает управление. В основном, это различные обработчики ошибок. Статические анализаторы, в отличие от динамических, не всегда способны отследить «судьбу указателя» на выделенную память и не обнаружат многие утечки памяти. С другой стороны, статические анализаторы проверяют весь код, независимо от вероятности его выполнения, и замечают ошибки. Таким образом, статические и динамические анализаторы дополняют друг друга.

Давайте посмотрим, какие утечки памяти я заметил в процессе разбора отчета, выданного PVS-Studio. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Отмечу также, что утечки памяти крайне неприятны для такого проекта, как Chromium, поэтому будет интересно о них поговорить. Согласно CWE эти ошибки можно классифицировать как CWE-401.

Часть 1: забыли освободить память перед выходом из функции


Рассмотрим ошибку в коде Chromium. Для начала я покажу вспомогательную функцию BnNew, которая выделяет и возвращает обнулённый буфер памяти:

uint32_t* BnNew() {
  uint32_t* result = new uint32_t[kBigIntSize];
  memset(result, 0, kBigIntSize * sizeof(uint32_t));
  return result;
}

А теперь посмотрим на код, который может приводить к утечке памяти:

std::string AndroidRSAPublicKey(crypto::RSAPrivateKey* key) {
  ....
  uint32_t* n = BnNew();
  ....
  RSAPublicKey pkey;
  pkey.len = kRSANumWords;
  pkey.exponent = 65537; // Fixed public exponent
  pkey.n0inv = 0 - ModInverse(n0, 0x100000000LL);
  if (pkey.n0inv == 0)
    return kDummyRSAPublicKey;
  ....
}

Если выполнится условие (pkey.n0inv == 0), то происходит выход из функции без освобождения буфера, указатель на который хранится в переменной n.

Анализатор указывает на этот дефект, выдавая предупреждение: V773 CWE-401 The function was exited without releasing the 'n' pointer. A memory leak is possible. android_rsa.cc 248

Кстати, на этом утечки памяти, относящиеся именно к самому Chromium, заканчиваются. Зато их немало в используемых библиотеках. А пользователям все равно, будет течь память в библиотеках Chromium или в самом Chromium. Поэтому ошибки в библиотеках не менее важны.

Следующие ошибки относятся к движку WebKit. Начать нам вновь придётся со вспомогательной функции:

static CSSValueList* CreateSpaceSeparated() {
  return new CSSValueList(kSpaceSeparator);
}

Теперь код, содержащий ошибку:
const CSSValue* CSSTransformValue::ToCSSValue(....) const {
  CSSValueList* transform_css_value =
    CSSValueList::CreateSpaceSeparated();
  for (size_t i = 0; i < transform_components_.size(); i++) {
    const CSSValue* component =
        transform_components_[i]->ToCSSValue(secure_context_mode);
    if (!component)
      return nullptr;                              // <=
    transform_css_value->Append(*component);
  }
  return transform_css_value;
}

Если указатель component окажется нулевым, то функция завершит свою работу, и при этом произойдёт утечка памяти.

Анализатор PVS-Studio выдаёт предупреждение: V773 CWE-401 The function was exited without releasing the 'transform_css_value' pointer. A memory leak is possible. csstransformvalue.cpp 73

Давайте посмотрим ещё какую-нибудь ошибку, относящуюся к WebKit.

Request* Request::CreateRequestWithRequestOrString(....)
{
  ....
  BodyStreamBuffer* temporary_body = ....;
  ....
  temporary_body =
   new BodyStreamBuffer(script_state, std::move(init.GetBody()));
  ....
  if (exception_state.HadException())
    return nullptr;
  .... 
}

Если функция HadException() вернёт истину, то функция досрочно завершит свою работу. При этом никто не вызовет оператор delete для указателя, хранящегося в переменной temporary_body.

Предупреждение PVS-Studio: V773 CWE-401 The function was exited without releasing the 'temporary_body' pointer. A memory leak is possible. request.cpp 381

Остальные ошибки, которые я заметил в WebKit, ничем не отличаются от описанных, поэтому я не вижу смысла рассматривать их в статье и ограничусь просто списком сообщений анализатора.
  • V773 CWE-401 The function was exited without releasing the 'image_set' pointer. A memory leak is possible. csspropertyparserhelpers.cpp 1507
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. csspropertyparserhelpers.cpp 1619
  • V773 CWE-401 The function was exited without releasing the 'shape' pointer. A memory leak is possible. cssparsingutils.cpp 248
  • V773 CWE-401 The function was exited without releasing the 'shape' pointer. A memory leak is possible. cssparsingutils.cpp 272
  • V773 CWE-401 The function was exited without releasing the 'shape' pointer. A memory leak is possible. cssparsingutils.cpp 289
  • V773 CWE-401 The function was exited without releasing the 'shape' pointer. A memory leak is possible. cssparsingutils.cpp 315
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 1359
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 1406
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 1359
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 1406
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. cssparsingutils.cpp 1985
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 2474
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cssparsingutils.cpp 2494
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. atruledescriptorparser.cpp 30
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. atruledescriptorparser.cpp 57
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. atruledescriptorparser.cpp 128
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. csssyntaxdescriptor.cpp 193
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 1232
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 1678
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 1727
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 2036
  • V773 CWE-401 The function was exited without releasing the 'size_and_line_height' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 2070
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. computedstylecssvaluemapping.cpp 2070
  • V773 CWE-401 The function was exited without releasing the 'file_list' pointer. A memory leak is possible. v8scriptvaluedeserializer.cpp 249
  • V773 CWE-401 The function was exited without releasing the 'file_list' pointer. A memory leak is possible. v8scriptvaluedeserializer.cpp 264
  • V773 CWE-401 The function was exited without releasing the 'computed_style_info' pointer. A memory leak is possible. inspectordomsnapshotagent.cpp 367
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. cursor.cpp 42
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. content.cpp 103
  • V773 CWE-401 The function was exited without releasing the 'variation_settings' pointer. A memory leak is possible. fontvariationsettings.cpp 56
  • V773 CWE-401 Visibility scope of the 'font_variation_value' pointer was exited without releasing the memory. A memory leak is possible. fontvariationsettings.cpp 58
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. rotate.cpp 32
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. quotes.cpp 25
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. textindent.cpp 52
  • V773 CWE-401 The function was exited without releasing the 'list' pointer. A memory leak is possible. shapeoutside.cpp 35
  • V773 CWE-401 The function was exited without releasing the 'port_array' pointer. A memory leak is possible. v8messageeventcustom.cpp 127

Много? Много. При этом здесь выписаны только те сообщения, на которые мне хватило сил. Я быстро заскучал и просматривал подобные предупреждения очень поверхностно. Скорее всего, при более внимательном анализе отчета, ошибок в WebKit будет найдено гораздо больше.

Что это значит? Это значит, что у проекта WebKit проблемы с утечками памяти, с чем я этот проект и «поздравляю».

Теперь перейдём к проекту ICU и рассмотрим ошибку, найденную в нём.

UVector*
RuleBasedTimeZone::copyRules(UVector* source) {
    if (source == NULL) {
        return NULL;
    }
    UErrorCode ec = U_ZERO_ERROR;
    int32_t size = source->size();
    UVector *rules = new UVector(size, ec);
    if (U_FAILURE(ec)) {
        return NULL;
    }
  ....
}

Если при инициализации объекта типа UVector произойдёт некая ошибка, то это отразится на статусе, который помещается в переменную ec. Например, конструктор вернёт статус U_MEMORY_ALLOCATION_ERROR если не удастся выделить буфер памяти для хранения нужного количества элементов. Однако, независимо от того, удастся выделить память для хранения элементов или нет, сам объект типа UVector будет создан и указатель на этот объект будет помещён в переменную rules.

Если конструктор возвратит статус U_MEMORY_ALLOCATION_ERROR, то произойдёт выход из функции. При этом сам объект типа UVector удалён не будет, и возникнет утечка памяти.

Предупреждение PVS-Studio: V773 CWE-401 The function was exited without releasing the 'rules' pointer. A memory leak is possible. rbtz.cpp 668

Другие ошибки в библиотеке ICU также приведу просто списком.
  • V773 CWE-401 The function was exited without releasing the 'tmpSet' pointer. A memory leak is possible. uspoof_impl.cpp 184
  • V773 CWE-401 The function was exited without releasing the 'result' pointer. A memory leak is possible. stsearch.cpp 301
  • V773 CWE-401 The function was exited without releasing the 'values' pointer. A memory leak is possible. tznames_impl.cpp 154
  • V773 CWE-401 The function was exited without releasing the 'filter' pointer. A memory leak is possible. tridpars.cpp 298
  • V773 CWE-401 The function was exited without releasing the 'targets' pointer. A memory leak is possible. transreg.cpp 984
  • V773 CWE-401 The function was exited without releasing the 'instance' pointer. A memory leak is possible. tzgnames.cpp 1216
  • V773 CWE-401 The function was exited without releasing the 'uset' pointer. A memory leak is possible. rbbiscan.cpp 1276

Что ещё я заметил?

Библиотека Libwebm.
  • V773 CWE-401 The function was exited without releasing the 'new_frame' pointer. A memory leak is possible. mkvmuxer.cc 3513
  • V773 CWE-401 The function was exited without releasing the 'new_frame' pointer. A memory leak is possible. mkvmuxer.cc 3539

Библиотека SwiftShader.
  • V773 CWE-401 The function was exited without releasing the 'node' pointer. A memory leak is possible. intermediate.cpp 405
  • V773 CWE-401 The function was exited without releasing the 'node' pointer. A memory leak is possible. intermediate.cpp 443
  • V773 CWE-401 The function was exited without releasing the 'node' pointer. A memory leak is possible. intermediate.cpp 514
  • V773 CWE-401 The function was exited without releasing the 'rightUnionArray' pointer. A memory leak is possible. intermediate.cpp 1457
  • V773 CWE-401 The function was exited without releasing the 'unionArray' pointer. A memory leak is possible. intermediate.cpp 1457
  • V773 CWE-401 The function was exited without releasing the 'aggregateArguments' pointer. A memory leak is possible. parsehelper.cpp 2109

Наверное, это далеко не все ошибки, но их мне достаточно для демонстрации возможностей PVS-Studio и написания этой статьи.

Часть 1: рекомендация


Что объединяет все рассмотренные выше случаи? То, что ошибки стали возможны из-за ручного управления памятью!

Друзья, мы уже используем C++17. Хватит вызывать оператор new, помещать результат в обыкновенный указатель, а потом забывать его освободить. Стыдно же.

Больше никаких обыкновенных указателей и последующего ручного управления выделенным ресурсом! Давайте всегда использовать умные указатели.

Современный стандарт C++ предлагает такие умные указатели, как unique_ptr, shared_ptr и weak_ptr. В большинстве случаев будет достаточно одного unique_ptr.

Вернёмся, например, к этому некорректному коду:

const CSSValue* CSSTransformValue::ToCSSValue(....) const {
  CSSValueList* transform_css_value =
    CSSValueList::CreateSpaceSeparated();
  for (size_t i = 0; i < transform_components_.size(); i++) {
    const CSSValue* component =
        transform_components_[i]->ToCSSValue(secure_context_mode);
    if (!component)
      return nullptr;
    transform_css_value->Append(*component);
  }
  return transform_css_value;
}

Давайте перепишем его с использованием unique_ptr. Для этого нам, во-первых, нужно изменить тип самого указателя. Во-вторых, в самом конце требуется вызвать функцию release, чтобы вернуть указатель на управляемый объект и более его не контролировать.

Корректный код:

const CSSValue* CSSTransformValue::ToCSSValue(....) const {
  unique_ptr<CSSValueList> transform_css_value(
    CSSValueList::CreateSpaceSeparated());
  for (size_t i = 0; i < transform_components_.size(); i++) {
    const CSSValue* component =
        transform_components_[i]->ToCSSValue(secure_context_mode);
    if (!component)
      return nullptr;
    transform_css_value->Append(*component);
  }
  return transform_css_value.release();
}

Я не планирую учить в этой статье как использовать умные указатели. Этой теме посвящено множество хороших статей и разделов в книгах. Я лишь хотел показать, что код из-за изменений не стал более сложным и громоздким. Зато теперь сделать ошибку будет намного сложнее.

Не думайте, что именно вы справитесь с new/delete или с malloc/free и не сделаете ошибок. Разработчики Chromium делают такие ошибки. Другие разработчики делают. Вы делаете и будете делать такие ошибки. И не надо лишних мечтаний, что ваша команда особенная :). Пользуясь случаем, прошу менеджеров прочитать сейчас вот эту заметку.

Используйте умные указатели.

Часть 2: realloc


По моим наблюдениям, программисты иногда неправильно используют функцию realloc. Вот как выглядит классический паттерн ошибки, связанный с использованием этой функции:

p = realloc(p, n);
if (!p)
  return ERROR;

Обратим внимание на следующее свойство функции: If there is not enough memory, the old memory block is not freed and null pointer is returned.

Поскольку NULL будет записан в переменную p, которая хранила указатель на буфер, то теряется возможность освободить этот буфер. Возникает утечка памяти.

Правильным вариантом будет переписать код, например, так:

void *old_p = p;
p = realloc(p, n);
if (!p)
{
  free(old_p);
  return ERROR;
}

Не обошлось без подобных ошибок и в библиотеках, используемых в проекте Chromium.

Например, рассмотрим фрагмент кода в кодеке FLAC.

FLAC__bool FLAC__format_entropy_codi.....ce_contents_ensure_size(
  FLAC__EntropyCodingMethod_PartitionedRiceContents *object,
  unsigned max_partition_order)
{
  ....
  if(object->capacity_by_order < max_partition_order) {
    if(0 == (object->parameters =
               realloc(object->parameters, ....)))
      return false;
    if(0 == (object->raw_bits = realloc(object->raw_bits, ....)))
      return false;
    ....
}

Функция увеличивает размер двух буферов:

  • object->parameters
  • object->raw_bits

Если происходит ошибка выделения памяти, то функция досрочно завершает работу и возвращает значение false. При этом теряется предыдущее значение указателя, и произойдёт утечка памяти.

Анализатор PVS-Studio выдаёт здесь два соответствующих предупреждения:

  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'object->parameters' is lost. Consider assigning realloc() to a temporary pointer. format.c 576
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'object->raw_bits' is lost. Consider assigning realloc() to a temporary pointer. format.c 578

Аналогичные недоработки в проекте WebRTC.
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'self->binary_far_history' is lost. Consider assigning realloc() to a temporary pointer. delay_estimator.cc 303
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'self->far_bit_counts' is lost. Consider assigning realloc() to a temporary pointer. delay_estimator.cc 306
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'self->mean_bit_counts' is lost. Consider assigning realloc() to a temporary pointer. delay_estimator.cc 453
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'self->bit_counts' is lost. Consider assigning realloc() to a temporary pointer. delay_estimator.cc 456
  • V701 CWE-401 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'self->histogram' is lost. Consider assigning realloc() to a temporary pointer. delay_estimator.cc 458

Ошибок этого типа в Chromium, к счастью, очень мало. По крайней мере, намного меньше, чем я обычно встречаю в других проектах.

Часть 2: рекомендация


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

Поэтому я не буду торопиться рекомендовать совсем отказаться от неё. Иногда это будет неоправданно. Прошу только быть аккуратным с этой функцией и не забывать паттерн ошибки, который я описал выше.

Впрочем, очень часто в C++ вполне можно обойтись без этой функции и использовать контейнеры, такие как std::vector или std::string. Эффективность контейнеров существенно выросла в последние годы. Я, например, был приятно удивлён, когда увидел, что больше нет отличия по производительности в ядре PVS-Studio между самодельным классом строки и std::string. А ведь много лет назад самодельный класс строки давал около 10% прироста производительности анализатору. Больше такой эффект не наблюдается и стало возможным удалить свой собственный класс. Сейчас класс std::string совсем не тот, каким он был 10 лет назад. Эффективность существенно повысилась благодаря современным возможностям компиляторов по оптимизации и благодаря таким новшествам языка, как, например, конструктор перемещения.

В общем, не спешите засучить рукава и вручную управлять памятью с помощью функций malloc, realloc, free. Почти наверняка std::vector окажется для ваших задач не менее эффективен. При этом, использовать std::vector намного проще. Сделать ошибку также станет сложнее. К низкоуровневым функциям есть смысл вернуться только тогда, когда профилировщик покажет, что это действительно одно из узких мест в работе программы.

Спасибо всем за внимание. Приглашаю скачать и попробовать анализатор PVS-Studio.



Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Chromium: Memory Leaks.

Комментарии (9)


  1. datacompboy
    29.01.2018 12:55

    Эх, вообще вот это странное поведение realloc из той же серии что и break в switch'ах.
    Когда-то может это и было эффективно с точки зрения реализации API… когда «получили null? завершаем программу совсем» — то есть утечки не было, ибо вся память очищалась автоматически выходом.
    На текущий момент — это мина замедленного действия.
    Реально почти никогда не взрывается (ибо память чтоб закончилась включая оверкоммит — еще постараться надо), зато когда уж взорвётся — всегда очень больно.


  1. eao197
    30.01.2018 11:01

    За рекомендацию №1, вообще-то, нужно бить по рукам. Поскольку это лечение симптомов, а не болезни. Нормальная рекомендация должна включать в себя изменение сигнатур методов CreateSpaceSeparated и ToCSSValue. На вот такие:

    static unique_ptr<CSSValueList> CreateSpaceSeparated() {...}
    unique_ptr<const CSSValue> CSSTransformValue::ToCSSValue(....) const {...}

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

    То, что предлагаете вы — это заткнуть дырку в одном месте, но оставить ее во всех других местах, где могут вызываться CreateSpaceSeparated и ToCSSValue.


    1. Andrey2008 Автор
      30.01.2018 11:06

      С замечанием согласен.


    1. mayorovp
      30.01.2018 12:26

      Тем не менее, в вашем варианте рекомендация может быть ошибочно понята как «перепишите весь проект на умные указатели», после чего отброшена потому что времени на это нет.

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


      1. eao197
        30.01.2018 12:35
        -1

        «перепишите весь проект на умные указатели»
        Начать нужно с того, что unique_ptr — это всего лишь доведенный до ума старый auto_ptr. Говнокод, который не использовал auto_ptr раньше и не использует unique_ptr сейчас, нужно переписывать. Есть на это время или нет — это уже вопрос приоритетов: хотят ли разработчики иметь нормальный код без утечек или их устраивают потенциальные дыры в их коде.
        можно переводить на умные указатели слой за слоем, метод за методом
        Можно сперва поменять сигнатуру CreateSpaceSeparated. Это уже устранит кучу проблем везде, где используется CreateSpaceSeparated. А не только внутри ToCSSValue. Затем можно поменять и сигнатуру ToCSSValue. Вот вы и получите постепенный перевод, только с меньшим количеством проблем.

        Единственный разумный довод в пользу рекомендации Андрея Карпова может заключаться в том, что изменение сигнатур может поломать ABI. Но это не имеет отношения к «постепенному переводу». И не исправляет исходную проблему — возврат голых владеющих указателей.


        1. mayorovp
          30.01.2018 12:41
          +1

          Нужно, нужно его переписывать, не спорю. Вот только объясните менеджерам сначала что следующий релиз откладывается на два месяца потому что тут какие-то голые указатели обнаружились, и новые фичи в этот релиз не войдут…

          И еще не забывайте про конфликты в гите. Никому не хочется весь день разгребать ошибки слияния только потому что десять программистов решили переписать один и тот же кусок кода.

          Если кому-то дали задачу написать или исправить ToCSSValue — значит, надо исправлять ToCSSValue, а не API сторонних функций.


          1. eao197
            30.01.2018 12:46

            Вы уж определитесь, вы хотите поговорить о технических проблемах или социальных? Проблемы с менеджерами и конфликтами в git-е — это социальные проблемы организации процесса разработки. К которым можно еще и приплести, например, хз какой code style, в котором разрешено возвращать голые владеющие указатели. Или, скажем, отсутствие должного code review. Или неиспользование статических анализаторов.

            А вот наличие API, которое чревато утечками памяти из-за того, что возвращаются голые владеющие указатели — это техническая проблема. И хорошее ее решение было указано.

            Так вот, если вы хотите обсуждать социальные проблемы — то это не ко мне.


            1. mayorovp
              30.01.2018 12:48

              Я говорю о решении технических проблем с учетом возможных социальных.


              1. eao197
                30.01.2018 12:56

                Ключевое слово «возможных».

                Есть рекомендация Карпова, которую сложно назвать удовлетворительной, т.к. она не решает исходной проблемы. И которая сама чревата другими возможными проблемами. Например:

                const CSSValue* CSSTransformValue::ToCSSValue(....) const {
                  unique_ptr<CSSValueList> transform_css_value(
                    CSSValueList::CreateSpaceSeparated());
                  for (size_t i = 0; i < transform_components_.size(); i++) {
                    const CSSValue* component =
                        transform_components_[i]->ToCSSValue(secure_context_mode);
                    if (!component)
                      return nullptr;
                    transform_css_value->Append(*component);
                  }
                  return transform_css_value.get();
                }

                И есть другая рекомендация, которая решает и исходную проблему, и препятствует возникновению других ошибок.

                И вот вы, весь из себя такой опытный в социльных проблемах разработки, приходите и говорите: «а ведь вашу рекомендацию можно неправильно понять, тогда как...» При этом вы сами не знаете, есть ли возможность поменять сигнатуры нескольких методов или нет.

                Ну, OK. Если вы работаете в условиях, когда вам выдали таск пофиксить утечку памяти в ToCSSValue и ничего больше вы не можете (да и не хотите) сделать, то удовлетворяйтесь дальше своим знанием социальных проблем.

                Только это не отменяет неудовлетворительности первоначальной рекомендации господина Карпова.