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

    +118

    1. 1
    2. 2
    3. 3
    4. 4
    filename_size = strlen(dest_dir) + strlen(basename) + 6;
        ctx.mtl_file = (char *) malloc(filename_size);
        ctx.obj_file = (char *) malloc(filename_size);
        sprintf(ctx.mtl_file, "%s/%s.mtl", dest_dir, basename);

    Долго соображали, почему вылезает сегфолт во free...

    Запостил: someone, 18 Июля 2012

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

    • Тьфу. Это уже исправленный код. В оригинале было +5.

      Почему нельзя редактировать и удалять запощенный код?
      Ответить
      • Видимо чтобы комментарии не теряли актуальности после этих правок.
        Ответить
      • эта функциональность не нужна тебе
        Ответить
      • Долго соображал, почему вылезает сегфолт во free... а оказалось надо было комментарии открыть почитать :)
        Ответить
    • >>Долго соображали, почему вылезает сегфолт во free
      А запили расширение для файла другой длины - ещё подумаешь...

      З.Ы. Как заметил коллега: Если компилятор _за_такой_код_ иногда предупреждает, ябзатакое просто сразу бы пи#дил.
      Ответить
      • > сразу бы пи#дил
        Причем без предупреждения.
        Ответить
      • Предложите свой вариант.
        Ответить
        • Предполагаться захардкоренный путь к файлу или у одмина будет возможность настройки?
          Ответить
          • Это библиотека. Имя файла подаётся на вход.
            Ответить
            • Несколько удивляет:
              1) почему по кускам, а не полный путь весь и сразу?
              2) Для чего расширение зашито? Если что: ждите ща скомпилим и вышлем либу с другим расширением.
              Ответить
            • И да, это у вас интересно так, имя поля структуры сопряженно
              с именем расширения файла.
              А если завтра '.mtl2', '.xmtl', '.tlm', etc?
              Ну там потестить разные версии, функционал расширили/добавили/убавили.
              "Счастливого сопровождения!"(с)
              Ещё один вопрос, чем так различны
              ctx.mtl_file и ctx.obj_file, если вы в них храните один и тот же путь?

              P.S. Если мне не изменяет память с цпу, то:
              1) (char *) для malloc() не обязателен;
              2) при передаче строки целиком можно посмотреть в сторону strdup().
              Ответить
              • > если вы в них храните один и тот же путь
                Нам, имхо, просто не показана еще одна строчка в духе:
                sprintf(ctx.obj_file, "%s/%s.obj", dest_dir, basename);
                Ответить
              • > 1) почему по кускам, а не полный путь весь и сразу?

                Это библиотека для конвертации 3DS в OBJ. На вход функции передаётся имя исходного 3DS-файла, путь к выходной папке и имя желаемой пары файлов (MTL и OBJ) без расширения.

                Папка передаётся отдельно, потому что в неё, кроме MTL и OBJ, ещё может понадобиться писать текстуры.

                > (char *) для malloc() не обязателен;

                Только в C. В C++ обязателен.
                Ответить
                • > Только в C. В C++ обязателен.
                  А зачем вам, простите, malloc, sprintf и strlen в c++?
                  Ответить
                  • Затем, что я считаю, что код должен компилироваться, даже если переименовать файл из .c в .cpp.

                    Вдруг кто-то возьмёт и вправду в .cpp вставит. Прецеденты были.
                    Ответить
                    • > Вдруг кто-то возьмёт и вправду в .cpp вставит.
                      Его личные проблемы. Библиотеки, имхо, не копипастят в свои файлы, а подключают и используют. Поэтому достаточно написать *.h так, чтобы он нормально компилировался и в С и в С++. А *.c всегда компилировать в режиме си.
                      Ответить
                      • Не помню, разве cl.exe не определяет расширения самостоятельно?
                        Ответить
                        • думаю, когда людям хочется с99, cl.exe не лучший выбор для компиляции сишных проектов
                          Ответить
                          • http://connect.microsoft.com/VisualStudio/feedback/details/606707/vs2010-missing-c99-support
                            Thank you for submitting this issue. Unfortunately, we are already have our hands full with C++0x and other FE work. At this point, we do not have plans to add C99 support to the compiler.
                            Thanks,
                            Mark Roberts
                            Visual C++ Compiler Team
                            Ответить
                      • :shame: я вот на неделе нагло коммуниздил 4 файла из zlib/contrib/minizip себе в проект, чтобы распаковываться из памяти в память
                        правда я их не менял, как и их расширение
                        Ответить
                        • > :shame:
                          Да ну... был бы он оформлен как нормальная библиотека... а так почему бы не скопировать его к себе в 3rdparty/minizip... что я и сделал месяц назад, когда нужно было генерить zip'ки. (Тоже ничего не правил и не менял, ну разве что приложил readme с описанием что это, зачем это, и откуда оно добыто).
                          Ответить
                    • И это выходит, все фишечки С99 не использовать?
                      Ответить
                      • Уел.

                        Именно в этом коде действительно используется C99 - одна из причин, по которой приходится компилировать mingw вместо MSVC (другая причина - кроссплатформенность). Так что в крестовый код просто так всё равно не скопипастить.
                        Ответить
                • Прелюдию желательно сразу, дабы избегать ненужных вопросов и писанины.
                  Что-то мне подсказывает, что для решения данной задачи С вам не нужен.
                  Мне так представляется, что можно так str= ''path_to_3ds|dest_dir|mtl|obj'';
                  В либе парсим и собираем пути. А с учётом возможностей кое-каких плюсовых
                  либ, решения значительно упроститься.
                  Как точно подметили: «писать прикладнуху на С, действительно, не самая удачная идея.»
                  Тем более в винде.
                  Ответить
                  • Уж лучше на C, чем на крестах.
                    Ответить
                    • Судя по характеру файлов вы пишете что-то тридешное, а такие проекты на сегодняшний день ретко бывают малыми, разве удобно писать большой проект без ООП? С++ как раз с такими обьемами кода показывает свои приемущества.
                      Ответить
                      • Это небольшая вспомогательная DLL'ка для приложения на Java. Причём это очень тонкая обёртка над сишной lib3ds. Кресты здесь только дадут лишний оверхед и запутают код.

                        А если писать полностью приложение без рантайма Java/.NET - то тогда уж либо на C+GObject, либо на более вменяемом OO-языке, чем кресты. D, например, или там Vala.
                        Ответить
    • кстати, snprintf в С уже моветон?
      Ответить
      • snprintf/strncpy для неуверенных. Риальные посоны используют только sprintf/strcpy.
        Ответить
      • asprintf кошерен, но есть не везде...
        Ответить
        • Ну не такая уж проблема запилить его для платформ, на которых его нет.

          UPD: хотя да, проблема, надо строку парсить и т.п. Половину логики printf'а переписать придется ;(
          Ответить
          • у нас в конторе сишечка используется только для платформы, где malloc-то нет
            я вообще не понимаю, как на сишечке в 2012 году можно пилить что то приличное прикладное - я каждый раз плачу кровавыми слезами, когда открываю наши сишные проекты с миллионом глобальных extern и static переменных
            Ответить
            • Никто не запрещает писать на сишечке модульно. Не умеють-с, курвы.
              Ответить
              • да, тяжелое наследие диких 90-х

                в принципе вспомнил, писал как-то года 4 назад на сишечке новый проект, вполне добротно вышло
                Ответить
            • > прикладное
              Ну вот писать прикладнуху на С, действительно, не самая удачная идея.
              Ответить
          • Не надо.

            int asprintf(char **strp, const char *format, ...)
            {
            	int size;
            	va_list argptr;
            	
            	va_start(argptr, format);
            	size = vsnprintf(NULL, format, argptr);
            	va_end(argptr);
            	
            	*strp = (char *) malloc(size + 1);
            	
            	va_start(argptr, format);
            	vsnprintf(strp, format, argptr);
            	va_end(argptr);
            	
            	return size;
            }


            Обработка ошибок опущена.
            Ответить
            • int vsnprintf(
                 char *buffer,
                 size_t count,
                 const char *format,
                 va_list argptr 
              );
              If buffer or format is NULL, or if count is less than or equal to zero, these functions invoke the invalid parameter handler, as described in Parameter Validation. If execution is allowed to continue, these functions return -1 and set errno to EINVAL.

              http://msdn.microsoft.com/en-us/library/1kt27hek(v=vs.90).aspx
              Ответить
              • Пардон. В первом случае должно быть

                vsnprintf(NULL, 0, format, argptr);

                Во втором:

                vsnprintf(*strp, size, format, argptr);

                MS, как всегда, несовместим со стандартом (лечится сборкой с mingw и #define vsnprintf __mingw_vsnprintf). Вот что пишет стандарт C99:

                http://www.codecogs.com/reference/computing/c/stdio.h/printf.php?alias=snprintf

                Also, they return the number of characters printed (not including the trailing \0 used to end output to strings) or a negative value if an output error occurs, except for snprintf and vsnprintf, which return the number of characters that would have been printed if the size were unlimited (again, not including the final \0 ).
                Ответить
                • так ведь vsnprintf только в С99 стандартизовали
                  потому микрософт даже чесаться не будет соответствовать
                  Ответить
                • есть чо за встраивание mingw в студию, со свойствами компиляции, дебаггером и поэтессами?
                  Ответить
    • Вообще, всем спасибо за комментарии.

      Отрефакторю код, используя самописный аналог asprintf/g_strdup_printf, поверх стандартного C99-того vsnprintf.

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

      Жаль всё-таки, что нет стандартной функции malloc+s_сколько_нужно_printf.
      Ответить

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