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

    +27

    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
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    int enumDevices(DevInfo* &lst) {
        int count = 0;
        DevInfo* tmp = NULL;
        Device device;
        for (int i = 0; i < MAXDEVICES; i++)
            if (device = OpenDevice(i)) {
                count++;
                realloc(tmp, sizeof(DevInfo)*count);
                ReadInfo(device, &tmp[count-1]
            }
        if (count == 0) return 0;
        lst = new DevInfo[count];
        for (int i = 0; i < count; i++)
            lst[i] = tmp[i];
        free(tmp);
        return count;
    }
    
    //................
    
    DevInfo* list;
    int devcount = enumDevices(list);
    /* работаем со списком */
    delete[] list;

    Самому стыдно.

    Запостил: Vindicar, 06 Января 2014

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

    • Забыл CloseDevice(device); на строке 10, но говно не в этом.
      Ответить
      • ещё забыл скобки и точку с запятой в строке 9
        Нам всем ужасно интересно, что это за девайсы такие?
        Ответить
        • Девайсы - АЦП L-Card 154. Это была небольшая обертка вокруг либы от производителя (там тоже не фонтан, один руглиш в идентификаторах чего стоит).
          Ответить
    • Где здесь с++, Vindicar? Хардкорная сишка же, ну кроме delete[] ;)

      Строки 12-15 вместо lst = tmp чисто ради того, чтобы не удалять malloc'нутый кусок через delete[]?

      P.S. Я бы все-таки поюзал std::vector...
      P.P.S. А еще можно было бы сделать какой-нибудь device_info_iterator, возвращающий по одной инфе об устройстве за раз :)
      Ответить
      • int enumDevices(DevInfo* &lst)
        Ответить
        • А, &. Слона то я и не заметил.
          Ответить
          • В чём слон?
            http://www.codingstandard.com/section/8-1-type-names/
            Ответить
            • коммент был заминусован.
              Ответить
            • > В чём слон?
              Всего лишь в том, что я не заметил еще один признак крестов, помимо new[] и delete[], в этом коде :)

              В остальном же этот код обычная сишка (ну ок, С99, т.к. объявление переменной в цикле).
              Ответить
      • Ну вот я такой нуб что хз как это сделать правильно.
        Я даже не помню зачем я это делал - наверно чтобы удалять именно через delete[].
        Ответить
        • Ну, если не заморачиваться с оптимизацией, то я бы сделал так. DevInfo не слишком большое же?
          std::vector<DevInfo> enumDevices() {
              std::vector<DevInfo> devices;
              DevInfo info;
              for (int i=0; i<MAXDEVICES; i++) {
                  if (Device device = OpenDevice(i)) {
                      ReadInfo(device, &info);
                      devices.push_back(info);
                      CloseDevice(device);
                  }
              }
              return devices; // за счет NRVO тут, по идее, не будет копирования
          }
          
          void something() {
              std::vector<DevInfo> devices = enumDevices(); // и тут не будет копирования
              // работаем со списком ...
          } // а тут все само удалится
          Если не доверяешь оптимизатору, то можно так:
          void enumDevices(std::vector<DevInfo> &devices) {
              DevInfo info;
              for (int i=0; i<MAXDEVICES; i++) {
                  if (Device device = OpenDevice(i)) {
                      ReadInfo(device, &info);
                      devices.push_back(info);
                      CloseDevice(device);
                  }
              }
          }
          
          void something() {
              std::vector<DevInfo> devices;
              enumDevices(devices);
              // работаем со списком ...
          } // а тут все само удалится
          Ответить
          • UPD: DevInfo info забыл внести внутрь if'а.
            Ответить
            • UPD: И раз у нас девайс не RAIIшен, то по-хорошему надо бы try, чтобы при исключении в push_back или ReadInfo прога не забыла сделать CloseDevice...
              Ответить
              • Ну там не исключение, там везде код ошибки возвращается.
                Ответить
                • Но в push_back все же может быть исключение о закончившейся памяти, поэтому я бы подвинул его чуть ниже, под CloseDevice.
                  Ответить
              • Да, туплю я, ReadInfo же сишная походу, и в ней исключения маловероятны. Если в ReadInfo исключений не бывает, то можно и так:
                if (Device device = OpenDevice(i)) {
                    DevInfo info;
                    ReadInfo(device, &info);
                    CloseDevice(device);
                    devices.push_back(info);
                }
                Ответить
                • А если ReadInfo() не выполнится успешно, зачем закладывать в список заведомый мусор?
                  Ответить
                  • > А если ReadInfo() не выполнится успешно, зачем закладывать в список заведомый мусор?
                    Тогда как-то так, в оригинале же на это проверки не было:
                    if (Device device = OpenDevice(i)) {
                        DevInfo info;
                        bool success = ReadInfo(device, &info);
                        CloseDevice(device);
                        if (success) {
                            devices.push_back(info);
                    }
                    Ответить
    • > realloc(tmp, sizeof(DevInfo)*count);

      Примем, что realloc() всегда возвращает tmp. Тогда можно не присваивать его значение в tmp, ибо оно есть постоянно.
      Ответить
      • Оно иногда еще и NULL может вернуть ;) Но здесь, походу, просто повезло: других аллокаций в этой функции нет, и realloc всегда отрабатывал не двигая массив.
        Ответить
        • Гм, в справке он вроде void возвращет...
          Ответить
          • О_о. А что за компилер?
            Ответить
          • ISO/IEC 9899:1999 7.20.3.4 The realloc function

            Synopsis
            #include <stdlib.h>
            void *realloc(void *ptr, size_t size);
            Description
            2. The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size. The contents of the new object shall be the same as that of the old object prior to deallocation, up to the lesser of the new and old sizes. Any bytes in the new object beyond the size of the old object have indeterminate values.

            3. If ptr is a null pointer, the realloc function behaves like the malloc function for the specified size. Otherwise, if ptr does not match a pointer earlier returned by the calloc, malloc, or realloc function, or if the space has been deallocated by a call to the free or realloc function, the behavior is undefined. If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged.

            Returns
            4. The realloc function returns a pointer to the new object (which may have the same value as a pointer to the old object), or a null pointer if the new object could not be allocated.
            Ответить
          • Вот так - смог бы:
            void (*realloc)(void * /*ptr*/, size_t /*size*/); /* Счастливой отладки :) */
            Ответить
          • > он вроде void возвращет
            указатель на void, а это совсем другое
            Ответить
            • Гм. А может это я слепошарый.
              Ответить
              • Да бывает, там звездочка прижата к правому краю, уставшими глазами можно не заметить.
                Ответить
    • Если проект будет жить дольше пары месяцев, рекомендую быстренько прочитать The C Programming Language (она интересная и короткая) и перейти на голую сишку, если есть такая возможность.
      Ответить
      • антикрестовая пропаганда?
        проплачено госдепом!
        Ответить
        • Нет, просто трезво оцениваю ситуацию... чтобы свободно писать на крестах, желательно прочитать два-три книжки мейерса + талмуд страуструпа и наступить на несколько десятков граблей. Когда слабо знаешь и C, и С++, а сроки поджимают, имхо лучше быстренько освоить сишку и писать на ней (хотя струструп считает иначе).
          Ответить
          • если смотреть на ОП, то тут не столько говна от незнания какого-то языка, сколько от рахитектуры
            вместо того, чтобы насоветовать человеку разобраться с декомпозицией и инкапсуляцией, ты его ещё больше сваливаешь в рутину ручного заката солнца - чтобы тактика съела вообще всю стратегию

            лично мое мнение, что написать хорошую программу на С ничуть не легче, и скорее всего, ещё даже сложнее, чем хорошую программу на С++
            при этом она будет отличаться явными чертами ООП (привет, ООП-ненавистники), но заодно содержать кучу низкоуровневого кода, засоряющего эфир - т.е. она будет более трудоёмка
            Ответить
            • Я поддерживаю Романа. Пусть сначала начнет писать на крестах как на сишке, а потом когда прочитает сотню толмудов продолжит писать на крестах как на крестах
              Ответить
              • неужели вам так сложно дались классы и несчастные контейнеры из stl?
                классы пригодятся в любом языке, даже в прости господи дельфях
                ну а без контейнеров - что, постоянно юзать сишные массивы или велосипедить списки? или сразу забить на std::string - "поебись ка сперва, как наши деды поеблись"!
                про буст тут пока что никто и не заикается

                человек посмотрит на эту унылость и съебёт в сишарп, скажет спасибо досвиданья
                Ответить
                • >неужели вам так сложно дались классы и несчастные контейнеры из stl?
                  Нет, тонкие моменты. Думаю я до сих пор не все знаю. А если ты сейчас меня попросишь назвать все тонкие моменты - я все не вспомню.
                  Ответить
                  • "во многой мудрости много печали, и кто умножает познания, умножает скорбь"
                    Ответить
                • > человек посмотрит на эту унылость и съебёт в сишарп, скажет спасибо досвиданья

                  так ведь скатертью дорожка :)
                  Ответить
                  • Как будто что-то плохое.
                    Ответить
                    • Наоборот, исключительно желаю счастья на новом поприще. Меньше потом разгребать.
                      Ответить
                      • то есть нормальные люди на шарпе не кодят?)
                        Ответить
                        • Наверно, он имел в виду, что на шарпе меньше говна пишут. Наверно так.
                          Ответить
                        • Причём здесь нормальные/ненормальные люди?
                          Есть люди, которым хочется работать с технологией Blurb, а есть те, кому не хочется (+те, кому не хочется, но заставляют, но мы пока их не рассматриваем). Я прекрасно понимаю людей, которым не нравится писать на крестах/сишке. Зачем истязать себя и окружающих? Нужно заниматься тем, что приносит удовольствие.
                          Ответить
                          • просто это прозвучало так, будто "не знаешь плюсы - не программист"
                            Ответить
                            • > просто это прозвучало так, будто "не знаешь плюсы - не программист"

                              У вас детская трамва и комплекс неполноценности? Хотите об этом поговорить?
                              Обратитесь к M-x doctor
                              Ответить
                              • Да нет, мне просто скучно

                                А вы хотели поговорить о неполноценности?)
                                Ответить
                                • Кегги, как тебе мой чеккер? Я старался.
                                  Ответить
                                  • Ссыль битая, дай нормальную, гляну. Мой допиливал или на дельфинариуме катал?
                                    Ответить
                                    • http://rghost.ru/51451820 - эта не битая. бинарник безопасен, я ручаюсь.
                                      На дельфях.
                                      Ответить
                                      • че мне эксешник, код давай
                                        Ответить
                                        • Сегодня уже не смогу, ибо комп недостижим. Завтра непременно выложу.
                                          Ответить
                                          • ну тады я в ASP.NET MVC ушел
                                            Ответить
                                            • А я купаться и спать. Сегодня всю бороду себе выщипал за регулярками - теперь как ощипанный петух.
                                              Ответить
                              • Нет, у вас комплекс сверхценности.
                                Ответить
                                • Да нафиг я никому не сдался. Если бы меня не было, ничего бы не изменилось.
                                  Ответить
                                  • изменилось бы самое важное для тебя - тебя бы не было)
                                    Ответить
                                    • > изменилось бы самое важное для тебя - тебя бы не было)

                                      Как это могло бы быть для него важным или не важным, если бы его не было?
                                      Ответить
                                      • В его теперешнем положение это имеет смысл. А если бы его не было - то ни имело бы) На самом деле это софистика.

                                        С какой то точки зрения что то имеет смысл, с другой - ничего не имеет и все эти точки зрения правы.
                                        Ответить
                                        • > В его теперешнем положение это имеет смысл.
                                          Ссылка на самого себя не спасёт от зловещего GC...

                                          Пока тебя и твои слова помнят другие люди - ты еще не умер ;)
                                          Ответить
                                          • Суть не в этом. Когда ты умрешь будет не важно, что думаю о тебе остальные. Да и сейчас не важно
                                            Ответить
                                            • >>не важно, что думаю о тебе остальные. Да и сейчас не важно
                                              Тебя случайно не из будущего к нам закинули, мсье Терминатор Кёгдан?
                                              Вспомнил, как один 27 летний ебень учил меня уму-разуму: никогда не спорь ни с кем, просто делай то, что тебе хочется.
                                              Ответить
                                              • Из прошлого. Я первый советник самого Токугавы

                                                Гугли, разрешаю
                                                Ответить
                                            • >будет не важно, что думаю о тебе остальные
                                              >сейчас не важно

                                              Это так не работает. Мысль принято доносить.
                                              Ответить
                                          • > Пока тебя и твои слова помнят другие люди - ты еще не умер ;)
                                            Нет, это слабая ссылка.
                                            Ответить
                                    • Если бы тебя не было, то для тебя теперешнего это не имеет значения.
                                      Ответить
                            • >> Меньше потом разгребать
                              > не знаешь плюсы - не программист

                              я имел в виду, что в плюсах очень много возможностей, некоторые из которых откровенно неудачны или унаследованы от сишки и плохо согласуются с остальным языком. Поэтому написать utter crap на плюсах без должного опыта и дисциплины очень просто. Некоторые вещи могут на первый вгляд выглядеть хорошей мыслью (вроде throws деклараций), но на самом деле вредны. Поэтому чем меньше неосведомлённые о подводных камнях люди будут писать на крестах, тем меньше потом разгребать.
                              Ответить
                              • Ну тогда я полностью с этим согласен. Как говориться - не умеешь - не берись
                                Ответить
                            • То есть их всего джва.
                              Ответить
                • >"поебись ка сперва, как наши деды поеблись"!
                  Тарас же сказал: это как армия и её нужно пройти чтобы быть настоящим мужикомпрограммистом.
                  Ответить
                • Человек уныло посмотрел на плюсы и сьебал на шарп - тру стори
                  Ответить
              • Stroustrup: We can’t just blame the beginners/novices/students, though. The presentation and teaching of C++ has been a constant problem. Almost a decade ago, when I first was to teach programming to freshman engineering students, I looked at the textbooks using C++ and despaired. More precisely, I did not despair, I was furious! There were (and are) books teaching every little obscure detail of C before getting to the far-easier-to-use C++ alternatives, and deeming those alternatives “advanced” to scare off all but the most determined student. Seriously, how could a standard-library vector be as hard to use well as a built-in array? How could using qsort() be simpler than using the more general and efficient sort()? C++ provides better notational support and stronger type checking than C does. This can lead to faster object code.
                Ответить
            • > сколько от рахитектуры
              ну а где тут особая рахитектура-то
              Считываем неизвестное кол-во объектов в память и работаем с ними. Никакой рахитектуры, просто незнание стандартных идиом работы с контейнерами и объектами. Без приличной литературы ведь всё равно будет что-то вроде
              using namespace std;
              vector<DeviceInfo> * devices = new vector<DeviceInfo>;
              enumDevices(devices);
              /* process devices */
              delete devices;
              Ответить
              • дык притом
                1. где класс Device, у которого есть open, close, readinfo
                2. почему девайсы тупо перенумерованы - уже странновато (могу предположить, что это, например, N устройств на rs-485 линии, или ip-устройства с адресом [base_ip + i])
                3. что произойдет дальше с девайсом? с ним работать кто-то собирается? т.е. его надо будет снова открывать, но уже для работы? неоправданные тормоза
                4. (со звёздочкой) девайс - по умолчанию сущность асинхронная, потому их бы желательно параллельно независимо попердолить, асихронно собирая результат об ответе или неответе на что-либо - сейчас я вангую, что если очередной девайс не отвечает, то все будут сосать в очереди, пока не наступит таймаут этого девайса - а он, допустим, 1..10 секунд

                p.s. пациент уже знает про ссылки, так что сможет передать вектор уж не по указателю
                Ответить
                • 1. Ну про девайсы мы ничего не знаем, может это внешнее апи (как минимум, там другая схема именования функций). Разумеется, неплохо бы запилить плюсовую RAII обёртку, но, судя по окружению, до такой идеоматики ещё далековато.
                  2, 3. Ждём ответа от топикстартера
                  4. Да, из общих соображений асинхронность при опросе устройств бы не помешала. С другой стороны, если "девайсы" лежат где-то на HDD (в моей либе у меня FAT-like файлы назывались девайсами), то профита от неё никакого.
                  Ответить
                • 1. Класс Device я сам и пишу, АПИ от производителя сугубо процедурное. ОП-код из static метода для получения списка доступных устройств.
                  2. Нумерация - честно, хрен его знает, брал с примера от вендора. Поищи L-Card E-154, найдешь. Подрубается оно по USB.
                  3. Да, потом будет использоваться только один.
                  Впрочем, на практике получение всего списка будет использоваться только один раз, чтобы увидеть серийники доступных АЦП и вбить нужный в настройки программы. Хотя похожий перебор используется для поиска девайса по серийнику, так как VIP/PID у них у всех одинаковые.
                  4. Асинхронности в открытии вроде нема, или я её не увидел, только при сборе данных. Всё по старинке, всё синхронно.
                  Ответить
                  • > Класс Device я сам и пишу
                    Какой же он класс, если вся работа с ним ведется по-сишному, внешними функциями... Он больше на какой-то хендл похож...
                    Ответить
                    • Эм, внешние функции? Ты про OpenDevice/ReadInfo/сотоварищи? Так это родное АПИ, процедурное.
                      А код, который я привел - кишки моей объектной обертки.
                      Ответить
                      • > OpenDevice/ReadInfo/сотоварищи?
                        Ага, именно их.

                        > А код, который я привел - кишки моей объектной обертки.
                        Понятно.
                        Ответить
          • Страуструп считает что:

            C++ was and is meant to be a tool for professionals and for people who takes programming seriously. Basically, C++ was not primarily designed for tasks of medium complexity, medium performance, and medium reliability, written by programmers of medium skills and experience.
            Ответить
        • Нехристи, кругом враги и нехристи...
          Ответить
          • Некрести, кругом враги и некрести...
            Ответить
            • Пришло время новых Крестовых походов.
              Ответить
              • И как всегда - с одно стороны пеноротые фанатики - с другой - всякие индусы)
                Ответить

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