1. Си / Говнокод #13700

    +135

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    char* getID(){
  uint32_t id = calcuate_id(); // Непринципиально, сложные какие то вычисления
  static char str[16] = {0};
  sprintf(str, "%ud", id);
  return str;
}

    На самом деле кусок модуля к nginx. Вычисляетися целое число, преобразуется в десятичное представление....

    Запостил: klavdiev, 29 Августа 2013

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

    • ava tirinox 29.08.2013 14:47 # 0

      Разве безопасно возвращать указатели на локальные переменные? О_о
      Ответить

      • ava klavdiev 29.08.2013 15:11 # +3

        э.... на ЛОКАЛЬНЫЕ опасно )
        Ответить

      • ava bormand 29.08.2013 16:01 # +2

        А это и не локальная переменная ;)

        А вообще этот код не потокобезопасен. Да и в одном потоке могут возникнуть проблемы, если не задокументирован тот факт, что результат живет только до следующего вызова.
        Ответить

        • ava bormand 29.08.2013 16:12 # 0

          > если не задокументирован
          Хотя тогда результат могут и освободить с помощью free...
          Ответить

          • ava blackhearted 30.08.2013 15:42 # 0

            В сишечках есть множество способов.
            И указатель или нет - разница невелика.
            Ответить

            • ava bormand 30.08.2013 15:46 # 0

              > разница невелика
              Их отделяет всего один символ: & или *.
              Ответить

        • ava Psionic 30.08.2013 12:52 # −1

          Да и функцию неплохо бы задекларировать как возвращающую указатель на константную строку - на всякий случай, от дурака.
          Ответить

        • ava WGH 31.08.2013 23:08 # 0

          Что-то я сомневаюсь, что nginx треды использует.
          Ответить

      • ava roman-kashitsyn 29.08.2013 16:02 # +1

        Использование указателя на объект, сконструированный на автоматической памяти, после выхода этого объекта из объемлющей области видимости - UB. Со статическими объектами это UB только в случае многопоточного доступа.
        Ответить

        • ava bormand 29.08.2013 16:03 # 0

          Но в данном случае мы видим не автоматический объект, а обычный статик :)

          И аналогичной херни в той же libc полно...
          Ответить

        • ava bormand 29.08.2013 16:13 # 0

          P.S. А ну вот, пока я комментировал, Роман уже пофиксил коммент на "Со статическими объектами это UB только в случае многопоточного доступа" ;)
          Ответить

          • ava roman-kashitsyn 29.08.2013 17:06 # +2

            в ковнокодике 5.0 будет версионирование комментов и оповещения на websocket
            Ответить

    • ava myaut 29.08.2013 16:17 # +1

      Может косяк в %ud (который сам по себе бессмысленен, если мы не хотим получить d на конце), вместо которого надо использовать PRIu32?
      Ответить

    • ava klavdiev 29.08.2013 16:34 # −1

      Косяк в том, что статическая переменная инициализируется единожды. И если в первый вызов получим число 12345 и строку "12345", то во второй вызов с результатом 3 мы получим строку "32345".
      Остальное по поводу free и нежелательной многопоточности вполне документировано, я же написал, это кусок модуля для nginx
      Ответить

      • ava bormand 29.08.2013 16:39 # 0

        > 3 мы получим строку "32345"
        Проверяли? :) Вот лишнее d оно пишет, да. Но откуда там возьмется 2345?! printf же сам всегда терминирует строку нулем. Или в api nginx'овых модулей не нультерминейтед строки, и для него важен этот хлам после нолика?
        Ответить

        • ava klavdiev 29.08.2013 17:00 # 0

          проверяли, естественно.
          Ответить

          • ava bormand 29.08.2013 17:16 # 0

            Ну т.е. согласно API nginx'а этот getID должен возвращать не null-terminated строку, а строку дополненную до 16 символов нулями?
            Ответить

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