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

    +53

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    void _debugPrintWaveHeader(const char *path, const WAVHEADER &source_header)
    {
    	FILE *debug = fopen("path", "wt");
    	fprintf(debug, "chunkId    = %s\n", source_header.chunkId);
    	fprintf(debug, "chunkSize  = %u\n", source_header.chunkSize);
    	fprintf(debug, "format     = %s\n", source_header.format);
    	fprintf(debug, "subCh1Id   = %s\n", source_header.subchunk1Id);
    	fprintf(debug, "subCh1Size = %u\n", source_header.subchunk1Size);
    	fprintf(debug, "audioform  = %u\n", source_header.audioFormat);
    	fprintf(debug, "numChanels = %u\n", source_header.numChannels);
    	fprintf(debug, "sampleRate = %u\n", source_header.sampleRate);
    	fprintf(debug, "byteRate   = %u\n", source_header.byteRate);
    	fprintf(debug, "blockAlign = %u\n", source_header.blockAlign);
    	fprintf(debug, "bitsPerSam = %u\n", source_header.bitsPerSample);
    	fprintf(debug, "subCh2Id   = %s\n", source_header.subchunk2Id);
    	fprintf(debug, "subCh2Size = %u\n", source_header.subchunk2Size);
    	fclose(debug);
    }

    Начал разгребать один говнопроект... При отладке программа всё падает и падает, падает и не может остановится. Смотрю на код - все в норме, а потом, по прошествии нескольких часов...

    Запостил: GreatMASTERcpp, 25 Августа 2014

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

    • Как этот проект вообще работал ? =) Я только сегодня за него взялся, а уже 2 поста на говнокод.ру.
      Ответить
    • Проблема в fopen("path") ?
      Так это просто опечатка, не говно. Ну ещё флажки в fprintf смущают (id строковые с явным size, но выводятся как zero-terminated строки).

      > Как этот проект вообще работал ? =) Я только сегодня за него взялся, а уже 2 поста

      Пф, иногда мне кажется, что говнокод это необходимое но недостаточное условие коммерческой успешности типичного проекта.
      Ответить
      • Поправил патч, вот результат отладки:
        chunkId = RIFF¦d
        chunkSize = 812198
        format = WAVEfmt 
        subCh1Id = fmt 
        subCh1Size = 18
        audioform = 1
        numChanels = 1
        sampleRate = 44100
        byteRate = 44100
        blockAlign = 1
        bitsPerSam = 8
        subCh2Id =
        subCh2Size = 1686135156
        Так еще неизвестно сколько еще там таких опечаток =)
        Ответить
        • >RIFF¦d
          Ну собственно о чем Роман выше и писал.
          Ответить
          • возможно, в xxxSize вовсе не размеры соответствующих Id :)
            Во всяком случае, по выводу не похоже. Опять же, кто знает, какие типы у xxxSize, действительно ли там unsigned int :) Приличные компиляторы на такое ругаются.
            Ответить
            • chunkid - 4 байта без терминатора, инфа 146%. Отсюда и мусор в хвосте.

              format - тоже 4 байта ("WAVE"), а "fmt" в нем вылез от начала следующего чанка.
              Ответить
              • Точно, теперь и сам вижу. Спасибо. :)
                Ответить
                • Ну там самый обыкновенный TLV - 4 байта тип чанка, 4 байта длина данных, а дальше сами данные. Ну и в чанках, в зависимости от типа, могут лежать другие чанки или же какие-то данные.
                  Ответить
      • Да за примерами даже ходить не надо.
        ReadWaveHeaderResult TMainForm::ReadWaveHeader(const wchar_t* filename, WAVHEADER &header)
        {
        FILE *file = _wfopen(filename, L"rb");
        fread(&header, sizeof(WAVHEADER), 1, file);
        fclose(file);
        #if DEBUG_ON == 1
        _debugPrintWaveHeader("C:\\logs\\open_wa ve_func.txt", header);
        #endif
        if(header.audioFormat != 1)
        {
        return ReadWaveHeaderResult::bad_audioFormat;
        }
        if(header.numChannels != 1)
        {
        return ReadWaveHeaderResult::bad_numChannels;
        }
        if(header.byteRate != 8 && header.byteRate != 16)
        {
        return ReadWaveHeaderResult::bad_byteRate;
        }
        return ReadWaveHeaderResult::good_result;
        }
        Это достойно отдельного поста да не хочу спамить =) (заспамлю ж все)
        Ответить
        • Ну а что не так (кроме хардкодного лога, забивания на нормальные WideString'и и парсинга WAV в коде ФОРМЫ)? Просто прога, видимо, довольно узкоспециализированная. И автор не хочет париться с другими форматами (которых по ТЗ может вообще не быть).

          P.S. Хотя, конечно, проще бы прицепить либу и получать от нее готовые массивы семплов, и не трахаться с байтами и заголовками.
          Ответить
          • Тест на внимательность =) (Я тоже не сразу это заметил)
            Ответить
            • Я сдаюсь, чего я проглядел?

              P.S. Ну еще ошибки открытия/чтеия не обработаны...
              Ответить
              • Я долго втыкал почему byterate колбасит (она должна быть либо 8 либо 16, а там 44 тыщи и больше), а потом заметил, что они просто перепутали byterate и bitspersamle.
                Ответить
                • > перепутали byterate и bitspersamle
                  Бля, слона то я и не заметил ;)

                  P.S. А этот код вообще юзали? А то он, походу, совсем не рабочий.
                  Ответить
                  • Самое интересное, что программа реально работала, но постоянно вылетала с acc. violation или internal exception. Проблему в байтрейтом решили... тупо закомментировав:
                    Application->MessageBoxA(L"Количество бит на семпл олжно быть равно 8 или 16", L"Error", 0);
                    exit(0);
                    (Да, да - сообщение с опечаткой!)
                    Ответить
                    • Ну по идее вместо exit должен быть return (из функции загрузки данных), но эта программа не церемонится!
                      Ответить
                      • > но эта программа не церемонится!
                        Автор из пыхи с ее die(), видимо, пришел.
                        Ответить
                        • Дибилдер откладывает свой нестираемый отпечаток на логику пишущего.
                          Пару watchdog'ов вполне решат проблему с крахами...
                          Ответить
                          • Кресты тоже повлияли =)
                            Ответить
                          • > Пару watchdog'ов
                            Ага, вместо фикса багов подопрём систему вотчдогами... Особенно круто это будет смотреться в GUI программе. Бедный юзер...

                            P.S. Против самих собачек ничего не имею, они полезны. Особенно на серверах и в embedded.
                            Ответить
                          • Кстати, о вотчдогах и крахах.

                            Был у меня на работе проектик с терминалами. И я там немного накосячил... В общем он крашился после каждой отрисовки экрана, переподключался к серверу, тот перевысылал ему состояние... Все дико мерцало, но можно было даже выбирать пункты в меню и т.п. Отказоустойчивость, мать её.
                            Ответить
      • > id строковые с явным size, но выводятся как zero-terminated строки
        Автор не знал о модификаторе точности ;)
        Ответить
    • Меня больше удручает, что публичный полезный код, написанный талантливыми сотрудниками известных компаний учит плохому. Например
      https://github.com/martine/ninja/blob/master/src/deps_log.cc
      Возможно, я слишком придирчивый и вообще ничего не понимаю, но у нас на код-ревью я бы такое точно завернул.
      Ответить
      • Готовят к выходу в свет.
        Ответить
      • > учит плохому
        Писать свои велосипеды вместо встраиваемой СУБД?
        Ответить
        • using namespace std; в хедерах, периодический игнор RAII, мутная семантика владения, куча маджикнумберов, ну и просто код в целом довольно лапшевидный.
          Против великов для кастомного формата хранения я в общем-то против ничего не имею.
          Ответить
          • > Против великов для кастомного формата хранения я в общем-то против ничего не имею.
            Ну просто, как я понимаю, они пытаются замутить ACID для таблички зависимостей. С той же berkeley db или sqlite это заняло бы от силы десяток строк. И сэкономило бы кучу времени на разработку/отладку...

            Против кастомных форматов для хранения/транспорта я, конечно, ничего не имею. А вот когда этот формат начинает плавно обрастать атомарностью, конкурентностью, попытками спасти инфу после краша и т.п. - имхо, стоит задуматься.
            Ответить
            • berkeley - это зависимость, которую не очень приятно тащить. С sqlite в этом плане попроще, но производительность может пострадать по сравнению с сырым бинарным файлом. Да базы у него бывают обратно не совместимые. А ACID я там что-то особого не заметил (только миграцию между версиями).

              Кстати, код по ссылке иногда сегфолтится (редко), но никто что-то не спешит разбираться, почему.

              "Есть два метода создания программного обеспечения. Один из них — сделать программу настолько простой, что, очевидно, в ней нет недостатков. И другой, сделать приложение настолько сложным, что в нем не видно явных недостатков."
              -- Энтони Хоар
              Ответить
              • > но производительность может пострадать по сравнению с сырым бинарным файлом
                > flush the file buffer after every record
                Так что маловероятно... А если записи будут втыкаться не по одной, а пачками в транзакции (а они, походу, именно пачками и втыкаются) - то даже sqlite порвёт эту самоделку как тузик грелку.
                Ответить
                • мда. Понаберут олимпиадников...
                  Ответить
                  • > Понаберут олимпиадников...
                    Есть такое... Причем почему-то все думают, что "базы тормозят, бинарные файлы это круто", а сами начинают пилить свой WAL с блекджеком и flush'ами. Да что все, я и сам этим страдал...
                    Ответить
              • > А ACID я там что-то особого не заметил (только миграцию между версиями).
                // Set the buffer size to this and flush the file buffer after every record to make sure records aren't written partially.
                // Check that the expected index matches the actual index. This can only happen if two ninja processes write to the same deps log concurrently
                // An error occurred while loading; try to recover by truncating the file to the last fully-read record

                Ну х.з., короче. До полного ACID'а оно еще не дотянуло. Но атомарность, согласованность и надежность уже начали прослеживаться...
                Ответить
    • - Нет! Не пойду! Я знаю, что Вы там себе надумали! Вчера Вы сказали, что любите меня. И теперь решили, что то, что Вы меня выпороли вчера, - это неправильно?
      Ответить

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