1. C++ / Говнокод #4602

    +157

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    void Scene3D::DrawTriangle(const Point3D &A, const Point3D &B, const Point3D &C, const Color& color)
    {
            // ---------------------------------------------------------------------------------------
            // вспомогательные вычисления
            // нормаль
            const Vector3D& n = (B - A) ^ (C - A);
    
            // ...
    
            // центр треугольника
            const Point3D& medium = (A + B + C) / 3.0;

    Руки как-то привыкли const TypeName& variableName набирать в определении параметров методов.
    И случайно набралось такое (строки 6, 11)

    Операторы (+, -, ^, /) над векторами возвращают Vector3D, не const Vector3D&.

    Заметил только через полгода, и всё это время оно почему-то работало, и даже ворнингов не было.

    Но такое ведь не должно работать!
    Результат вычисления в правой части присваивания структура, то есть она возвращается в стеке. Если бы я присвоил её какой-то локальной переменной, для которой выделена память в стеке текущей функции, то она бы перед удалением скопировалась в локальную переменную. А так получается что ссылка (n, medium) указывает куда-то на стек, где временно хранится возвращенное оператором значение. И при следующем вызове любой функции эта область стека должна перезаписаться.

    Запостил: burdakovd, 12 Ноября 2010

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

    • Незнание стандарта — такое незнание...
      Если привязать временный объект к константной ссылке, время его жизни продлевается до тех пор, пока жива та ссылка. Так-то!
      Ответить
      • Хм. то есть такое тоже корректно?

        const int& q() {
            int x = 1;
            return x;
        }
        Ответить
        • Нет, такое некорректно.
          Здесь вы возвращаете адрес переменной, которая действительно будет уничтожена при выходе из q.
          Стандарт же позволяет продлить время жизни объекта, который окажется на стеке во фрейме вызывающего кода путем привязывания этого объекта к константной ссылке. Для этого вызываемый код должен объект по значению.
          Ответить
          • А можно конкретнее где такое написано?
            В данном случае ссылка использовалась только в пределах метода.

            А если вместо того чтобы создавать локальную константную ссылку я создал бы тут экземпляр какого-нибудь класса, который в конструкторе принимает константную ссылку и сохраняет её себе в поле?

            Этот объект присвоил бы какому-то полю this.

            Этот объект, а значит и ссылка, а значит и временный объект, на который указывает ссылка жили бы неопределенно долго. А ссылка-то указывает на участок памяти стека.

            То есть стек станет фрагментирован?
            P.S. - свободное место в стеке станет фрагментировано?
            Ответить
            • Тогда временный объект привязывается к константной ссылке, которая является аргументом конструктора. Она будет уничтожена по завершении конструктора, а вместе с ней и временный объект. То, что вы скопировали эти ссылку вовнутрь, уже не важно.
              Тут привязка идет только к той ссылке, которую инициализировали временным объектом, ее копирование ни на что не влияет. Тут же нет подсчета ссылок :)
              Читать надо в Стандарте, 12.2/5.
              Ответить
            • можешь еще вот это глянуть: http://stackoverflow.com/questions/2088259/literal-initialization-for-const-references и http://stackoverflow.com/questions/2604206/c-constant-reference-lifetime/2604269#2604269
              Ответить
    • Вообще, так писать некорректно.
      Хоть и правильный результат возможен, но только если область памяти, выше адркса аппаратного стека не запакостится.
      Сама по себе она, конечно, не поменяется, но добавление любой "безобидной операции", при котором происходит вызов функции - и результат будет непредсказуем.
      Ответить

    Добавить комментарий