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

    −23

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    6. 6
    label t1:
    if(something) {
    
    } else {
    goto t1;
    }

    Возвращение назад после провала, и так до бесконечности.... Примерно такой код за который вас забанят почти везде! Не повторяйте чужих ошибок, в то так ардуина сдохнет. Был такой случай, заела и все!

    Запостил: acterhd, 03 Апреля 2017

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

    • когда вижу "goto" всегда хочу руки поотрубать
      Ответить
      • Дык это для драммы, для остроты картины!
        Ответить
      • Пропущены знаки препинания!
        Ответить
      • > когда вижу "goto" всегда хочу руки поотрубать

        В сишечке goto — необходимое зло, единственный удобный способ освобождения нескольких ресурсов.
        Ответить
        • если кто-то юзает гото - значит он не знает как писать проги. всегда можно зающать while (true) { break; } на худой конец
          Ответить
          • Ты много на сишечке писал? Покажи, как нужно правильно освобождать 4 ресурса внутри одной функции с помощью while (true) { break; }.
            Ответить
            • напиши пример. я тебе прооптимизирую :)
              Ответить
              • Ну вот абстрактный типичный системный сишкокод
                ErrCode OpenDbReadonly(const char* root_path, MyDb** out) {
                  char* index_path = NULL;
                  char* storage_path = NULL;
                  FILE* index = NULL;
                  FILE* storage = NULL;
                  ErrCode code = OK;
                
                  index_path = PathConcat(root_path, INDEX_FILE_NAME);
                  if (!index_path) { code = NO_MEM; goto fail; }
                
                  storage_path = PathConcat(storage_path, STORAGE_FILE_NAME);
                  if (!storage_path) { code = NO_MEM; goto fail; }
                
                  index = fopen(index_path, "rb");
                  if (!index) { code = FS_ERR; goto fail; }
                  if (!ValidateIndex(index)) { code = INCONSISTENT; goto fail; }
                
                  storage = fopen(storage_path, "rb");
                  if (!storage) { code = FS_ERR; goto fail; }
                  if (!ValidStorage(storage)) { code = INCONSISTENT; goto fail; }
                
                  free(index_path);
                  free(storage_path);
                
                  *out = calloc(1, sizeof(MyDb));
                  (*out)->index = index;
                  (*out)->storage = storage;
                
                  return code;
                
                fail:
                  free(index_path);
                  free(storage_path);
                  if (index) fclose(index);
                  if (storage) fclose(storage);
                
                  return code;
                }
                Ответить
                • попытался твой пример более демонстративным сделать:
                  ErrCode
                  OpenDbReadonly(const char* root_path, MyDb** out)
                  {
                    char* index_path = NULL;
                    char* storage_path = NULL;
                    FILE* index = NULL;
                    FILE* storage = NULL;
                    ErrCode code = OK;
                  
                    index_path = PathConcat(root_path, INDEX_FILE_NAME);
                    if (!index_path) { code = NO_MEM; goto fail_end; }
                  
                    storage_path = PathConcat(storage_path, STORAGE_FILE_NAME);
                    if (!storage_path) { code = NO_MEM; goto fail_storage_path; }
                  
                    index = fopen(index_path, "rb");
                    if (!index) { code = FS_ERR; goto fail_open_index; }
                    if (!ValidateIndex(index)) { code = INCONSISTENT; goto fail_validate_index; }
                  
                    storage = fopen(storage_path, "rb");
                    if (!storage) { code = FS_ERR; goto fail_open_storage; }
                    if (!ValidStorage(storage)) { code = INCONSISTENT; goto fail_validate_storage; }
                  
                    free(index_path);
                    free(storage_path);
                  
                    *out = calloc(1, sizeof(MyDb));
                    (*out)->index = index;
                    (*out)->storage = storage;
                  
                    return OK;
                  
                  fail_validate_storage:
                    fclose(storage);
                  
                  fail_open_storage:
                  fail_validate_index:
                    fclose(index);
                  
                  fail_open_index:
                    free(storage_path);
                  
                  fail_storage_path:
                    free(index_path);
                  
                  fail_end:
                    return code;
                  }


                  goto убить никогда не получится - в особенности в системщине - потому что так процы работают. если знаешь как оно просто может быть в объектном коде - то писать горбатый код уже просто руки не подымаются. тем учитывая что горбатый код на порядки сложнее править и поддерживать.
                  Ответить
                  • > пример более демонстративным сделать

                    Ну у тебя "деструкторы" сложно заанроллены, здесь я не вижу особого смысла это делать. Со временем я пришёл к выводу, что гораздо проще писать "деструкторы", которые являются NOOP в случае, когда ресурс не был проинициализирован, как free(NULL). Тогда стек меток либо вообще не нужен, либо гораздо меньше, и функция гораздо проще для понимания.
                    Ответить
                    • > здесь я не вижу особого смысла это делать.

                      от приложения, к приложению.

                      на системщине это часто и секурити и производительность: ничего лишнего в пустую не делать.
                      Ответить
                      • > ничего лишнего в пустую не делать

                        Я бы сказал, что на фоне открытия файла/выделения памяти пара дополнительных сравнений указателей в случае, который почти никогда не происходит — это копейки в 95% случаев.
                        Ответить
                        • производительность - да. но безопастность - это другая песня. были в прошлом DDOSы основаные на флуде системы вызовами с дорогим освобождением ресурсов.

                          но в прикладном коде, да, 95% времени это не нужно.
                          Ответить
                  • OpenDbReadonly(const char* root_path, MyDb** out)
                    {
                    	char* index_path = NULL;
                    	char* storage_path = NULL;
                    	FILE* index = NULL;
                    	FILE* storage = NULL;
                    	ErrCode code = -1;
                    
                    	code = NO_MEM;
                    	index_path = PathConcat(root_path, INDEX_FILE_NAME);
                    	if (index_path)
                    	{
                    		code = NO_MEM;
                    		storage_path = PathConcat(storage_path, STORAGE_FILE_NAME);
                    		if (storage_path)
                    		{
                    			code = FS_ERR;
                    			index = fopen(index_path, "rb");
                    			if (index)
                    			{
                    				code = INCONSISTENT;
                    				if (ValidateIndex(index))
                    				{
                    					code = FS_ERR;
                    					storage = fopen(storage_path, "rb");
                    					if (storage)
                    					{
                    						code = INCONSISTENT;
                    						if (ValidStorage(storage))
                    						{
                    							*out = calloc(1, sizeof(MyDb));
                    							(*out)->index = index;
                    							(*out)->storage = storage;
                    							code = OK;
                    						}
                    					}
                    				}
                    			}
                    		}
                    	}
                    
                    	if (storage)
                    		fclose(storage);
                    
                    	if (index)
                    		fclose(index);
                    
                    	if (storage_path)
                    		free(storage_path);
                    
                    	if (index_path)
                    		free(index_path);
                    
                    	return code;
                    }


                    Настоятельно рекомендую юзать LIBUNWIND
                    Ответить
                    • Ты серьёзно считаешь, что этот вариант лучше?
                      Или исходишь из принципа "главное, чтобы не было goto, а то я где-то слышал, что он harmful"?
                      Понятно, что без goto можно переписать, просто от этого код становится хуже для понимания. В твоём случае он превратился в двумерный косяк птиц, которые летят на юг.
                      Ответить
                      • конечно он лучше. Он как минимум удовлетворяет требованиям "Clean code"

                        для справки

                        http://www.planetgeek.ch/wp-content/uploads/2014/11/Clean-Code-V2.4.pdf
                        Ответить
                        • > карго-культ

                          Ок, я так и думал.
                          Ответить
                          • >> Члены культа обычно не понимают в полной мере значимость производства или коммерции

                            Точно думаешь я не знаю "значимость производства или коммерции" :)?
                            Ответить
                            • Ты утверждаешь, что код без goto лучше, потому что в нём нет goto, потому-то где-то написано, что goto это плохо. Магическое мышление в чистом виде.

                              К слову, в той pdf-ке, которую ты привёл, слово goto вообще не встречается. Да и рассчитана она явно на ООП-языки, где есть альтернативные способы освобождения ресурсов, а не на сишку.
                              Ответить
                              • он не правильную пдф тебе дал:

                                http://ricardogeek.com/docs/clean_code.pdf
                                Ответить
                              • показать все, что скрытоunwind тоже религия не позволяет юзать :)?

                                ну или хотябы бейсик try/catch аналог

                                #include <stdio.h>
                                #include <setjmp.h>
                                
                                #define TRY do{ jmp_buf ex_buf__; if( !setjmp(ex_buf__) ){
                                #define CATCH } else {
                                #define ETRY } }while(0)
                                #define THROW longjmp(ex_buf__, 1)
                                
                                int
                                main(int argc, char** argv)
                                {
                                   TRY
                                   {
                                      printf("In Try Statement\n");
                                      THROW;
                                      printf("I do not appear\n");
                                   }
                                   CATCH
                                   {
                                      printf("Got Exception!\n");
                                   }
                                   ETRY;
                                
                                   return 0;
                                }
                                Ответить
                                • "Holy shit. For my own mental health I'm going to assume you're just messing with me and aren't actually retarded."
                                  Ответить
                                  • Зима. Горнолыжная трасса. На склоне @ASD_77 в полной экипировке: очки, шлем, модный костюм. Несется со склона и, подпрыгнув на трамплине, падает кубарем и врезается в дерево. Встает: палки погнуты, лыжи поломаны, костюм в клочья, очки разбиты, все лицо в крови и не хватает зубов.
                                    Отряхивается, подымает взгляд обратно на вершину спуска говорит: "Б.дь.! Все равно лучше, чем GOTO!"
                                    Ответить
                                    • показать все, что скрытода лучше. гото для лохов не умеющих проги писать. все. full stop :)
                                      Ответить
                                      • да я лучше себе глаза вилкой повыкалупую лиж бы гото не видеть :)
                                        Ответить
                                        • >>да я лучше себе глаза вилкой повыкалупую лиж бы гото не видеть :)

                                          Для этого миксер вроде юзают. причем давно уже.
                                          Ответить
                                  • I'am here.
                                    Ответить
                        • Укушенный радиоактивным Эдсгером Дейкстрой, ASD_77 превратился в человека-дейкстру:
                          -- Помогите, меня грабят!
                          -- Человек-дейкстра спешит на помощь! Goto considered harmful, до свидания.
                          Ответить
                        • вижу клин код - минусую.

                          > конечно он лучше. Он как минимум удовлетворяет требованиям "Clean code"

                          "в жопу людей которые с этим кодом работают - давайте лучше послушаем мнение случайных людей с интернета."
                          Ответить
                          • > вижу клин код

                            А, так его так назвали потому, что он выглядит как клин уток, летящих на юг?
                            Ответить
                            • > клин уток, летящих на юг?

                              нет, на север. это же суицидальные утки.
                              Ответить
                      • я показал что гото нах не надо это раз

                        ненравятся "птицы" юзай libunwind или аналоги для error хандлинга
                        Ответить
                      • На моём мониторе они летят на восток.
                        Срочно проведите над Романом экзорцизм -- он опять по стенам ходит!
                        Ответить
                        • > На моём мониторе они летят на восток.
                          Так ты разверни стол вокруг своей оси, сверяясь по компасу.
                          Ответить
                    • убью человека, который сунет мне в проект дерьмо с 7-м уровнем вложенности.

                      Даже после каждого if выделять память и делать early return лучше
                      Ответить
                      • освобождать*
                        Ответить
                      • а если с 8-м? а если рекурсия будет?
                        Ответить
                      • только не плач :)

                        OpenDbReadonly(const char* root_path, MyDb** out)
                        {
                        	char* index_path = NULL;
                        	char* storage_path = NULL;
                        	FILE* index = NULL;
                        	FILE* storage = NULL;
                        	ErrCode code = -1;
                        
                        	code = NO_MEM;
                        	index_path = PathConcat(root_path, INDEX_FILE_NAME);
                        
                            while (true)
                            {
                                if (!index_path)
                                {
                                    code = NO_MEM;
                                    break;
                                }
                        
                                storage_path = PathConcat(storage_path, STORAGE_FILE_NAME);
                                if (!storage_path)
                                {
                                    code = FS_ERR;
                                    break;
                                }
                        
                                index = fopen(index_path, "rb");
                                if (!index)
                                {
                                    break;
                                }
                        
                                code = INCONSISTENT;
                                if (!ValidateIndex(index))
                                {
                                    code = FS_ERR;
                                    break;
                                }
                        
                                storage = fopen(storage_path, "rb");
                                if (!storage)
                                {
                                    code = INCONSISTENT;
                                    break;
                                }
                        
                                if (!ValidStorage(storage))
                                {
                                    *out = calloc(1, sizeof(MyDb));
                                    (*out)->index = index;
                                    (*out)->storage = storage;
                                    code = OK;
                                }
                        
                                break;
                            }
                        
                        	if (storage)
                        		fclose(storage);
                        
                        	if (index)
                        		fclose(index);
                        
                        	if (storage_path)
                        		free(storage_path);
                        
                        	if (index_path)
                        		free(index_path);
                        
                        	return code;
                        }
                        Ответить
                        • Поздравляю, Вы написали код с goto!
                          Ответить
                          • Формально не по Дейкстре.

                            Фактически программирование с блоком. Прыгнуть можно только в его конец. Возможности наговнять лапши уменьшаются.

                            С goto можно прыгать куда угодно ­— он так явно не отделяет скоуп работы от скоупа освобожения ресурсов.
                            Ответить
                      • Ну как, убил?
                        Ответить
                    • Даже в мире JS уже вовсю используют промисы, чтобы не плодить вложенной в мире JS уже вовсю используют промисы, чтобы не плодить вложенной в мире JS уже вовсю используют промисы, чтобы не плодить вложенной питушни.
                      Ответить
                • *out = calloc(1, sizeof(MyDb));
                    (*out)->index = index;
                    (*out)->storage = storage;

                  а если тут память кончится?
                  if (!storage_path) { code = NO_MEM; goto fail; }
                  fail:
                    free(storage_path);

                  Зачем free(0)?

                  я бы, кстати, не разделял освобождение ресурсов позитивного и негативного сценариев.
                  Ответить
                  • > а если тут память кончится?
                    Да, пропустил. Мысленно добавляем if (!*out) { code = NO_MEM; goto fail; }

                    > не разделял освобождение ресурсов позитивного и негативного сценариев.
                    Спорный вопрос, так легко превратить функцию в нечитаемую непонятную лапшу из переходов.

                    > Зачем free(0)?
                    http://govnokod.ru/22731#comment380554
                    Ответить
                    • > Спорный вопрос, так легко превратить функцию в нечитаемую непонятную лапшу из переходов.

                      За это и люблю плюсы. Кстати, а если так?: http://ideone.com/CyNoSl
                      Ответить
                      • а вдруг тебе 32кб оперативы не хватит на хранение массива. ты что это же такой крутой мемори лик будет :)
                        Ответить
                      • > Кстати, а если так?

                        Так это же изоморфно гошному defer :)
                        Только делитеры нужно вызывать в обратном порядке, а не в прямом, иначе будет беда.

                        Ещё похоже, что нужно два стека делитеров: на временные объекты, которые удаляются в любом случае, и на "постоянные", которые покидают скоп функции транзакционно.
                        Ответить
                        • постоянные можно не закидывать в список к удалению
                          Ответить
                    • в gcc можно еще делать nested function для освобождения, звать её и делать early return
                      Ответить
                  • > не разделял освобождение ресурсов
                    Иногда, после позитивного сценария, часть этих ресурсов вообще должна остаться в живых - их вернут, пристегнут к к какой-то структуре и т.п. Так что не всегда можно смешать.
                    Ответить
                • > ErrCode code = OK; // в начале функции
                  > return code; // после очистки ресурсов при ошибке
                  Опасный паттерн, можно случайно вернуть успех:
                  http://govnokod.ru/14935

                  З.Ы. Хотя, конечно, они там просто с однострочниками довыябывались.
                  Ответить
                  • Да, можно какой-нибудь UNKNOWN по дефолту выставлять.
                    Ответить
                    • Ну я бы ещё в успешной ветке OK явно вернул - мало ли что там в code окажется к концу этой ветки (какую-нибудь минорную ошибку только что получили и заигнорили и т.п.).
                      Ответить
                    • З.Ы. И один фиг, можно забыть засунуть код в code (и там окажется OK от предыдущего вызова) или тупо забыть обработать ошибку...

                      Обработка ошибок в си - боль.
                      Ответить
                      • тебе не нужно обрабатывать ошибки, если ты их не генерируешь
                        [картинка с негром здесь]
                        Ответить
                      • Бормандяшу заминусили :'<
                        Ответить
                • Чем хуже было бы написать какой-то
                  void clearShit(...) {
                    free(index_path);
                    free(storage_path);
                    if (index) fclose(index);
                    if (storage) fclose(storage);
                  }

                  И потом вызывать его и сразу делать return? И говнопереназначений code не было бы, можно сразу возвращать литерал
                  Ответить
                  • index_path, storage_path и прочие — это локальные переменные, их няльзя очистить во внешней функции, не передавая в неё (а вносить в прототип все локальные переменные и потом в каждом вызове их перечислять — то ещё говнище будет). Замыканий и/или локальных функций в сишку без гнусных расширений ня завезли, методов тоже.
                    Ответить
                    • Тогда сделаем структуру с этим говном и будем его передавать
                      Ответить
                      • Ну вот уже и структуры на ровном месте... Чем бы дитя не тешилось, лишь бы goto не писало?
                        Ответить
                        • > Ну вот уже и структуры на ровном месте

                          Так и до метатаблиц недалеко...
                          Ответить
                        • >структуры на ровном месте

                          я понимаю что байтоебы удавятся за три байта при том что открывают файл в том же месте, но давайте смотреть на вещи объективно

                          goto:
                          - низкий cohesity. Ты определяешь какую-то брухлю из освобождений посреди функции, у тебя может быть спереди код, сзади код, от goto 100 строк и ты как опездол бегаешь по функции чтобы что-то понять
                          - Ты перед тем как в эту брухлю что-то добавить должен пройтись по всем меткам (сверху энтузиасты goto "улучшили" код до 6 меток)
                          - Происходит какой-то мутный ассайн кода перед переходом а если не сделаешь то обосрешься или говно вернешь
                          - хуевая расширяемость. Если ты вдруг решишь что после очистки ресурсов можно зарекавериться то что, goto обратно напишешь?

                          функция с аргументами или структурой:
                          - чуть больше кода на структуру или педерачу аргументов
                          - оверхед на вызов функции
                          - Высокая cohesity. Обосрался, сразу вызвал функцию, сразу сделал return
                          - Нет проблем с реассайном. Сразу делаешь return FS_ERR, риск обосраться 0 процентов
                          - Гораздо лучше масштабируется. Если увелится кол-во кода, можно еще что-то добавить в функцию, скомпозировать функции или сделать новые. Фанаты гото конечно же сейчас просрутся 20 метками и расскажут как им охуенно
                          Ответить
          • > всегда можно зающать while (true) { break; } на худой конец

            Заминусили пацана ни за что.
            Идиоматичнее конечно:
            do {  
                  ... 
                  break; 
                  ... 
            }while(0);
            Но по факту можно и так прыгать в конец, без goto.
            Ответить
      • Юзаю гото
        Даже иногда в сишарпике
        Ни о чем не жалею
        Ответить
        • Нечего терять?
          Ответить
          • Терять-то есть что
            Ключи от дома, например. Или от говнокода
            Ответить
          • Ты же сказал что съебуешь с сайта с концами????
            Ответить
            • Пруф?
              Ответить
              • Выслал пруф
                Проверь
                Ответить
                • Проверил. Ты пиздун.
                  Ответить
                  • Настоящий борманд ушел с сайта ещё в 2014 году, с того момента мы общались с клонами борманда. Они разного роста, разной степени ботоксности, с гладким и морщинистым лбом, с разными пропорциями носа, подбородка и т.д., с разными голосами. Один умеет нырять за амфорами, другой указательным пальцем с ошибками играть на пианинах, третий летает с журавлями. Но все они почему-то забыли германский язык, все разного возраста, иногда даже доходит до того, что в киоске союзпечати лежат рядом разные журналы с разными бормандами ))) Вот такой сайт, вот такая анонимность.

                    А какая разница, один он или его много? Борманд в России и в мире, это уже давно не человек. Борманд, это миф. Он не то, что представляет из себя на самом деле, это то, что создала целенаправленная пропаганда, СМИ, всевозможные имиджмейкеры. Он не то, что он есть, он то, что в нем хотят видеть. Он иллюзия, мираж миллионов.
                    Ответить
                    • Ну кто так дословно цитирует, где адаптация под ГК?

                      Настоящий борманд няпокашнулся ещё в 2017 году, с того момента мы общались с клонами борманда. Они разного роста, разным цветом волос на аватарке, с разными пропорциями ушек, с разными голосами. Один умеет писать на С++, другой указательным пальцем с ошибками играет в осу, третий говнокодит под контроллеры. Но все они почему-то забыли паскаль, все разного возраста, иногда даже доходит до того, что в тредах на ГК лежат рядом разные комменты с разными бормандами ))) Вот такой сайт, вот такая анонимность.
                      Ответить
                      • > иногда даже доходит до того, что в тредах на ГК лежат рядом разные комменты с разными бормандами )))

                        Подтвярждяю ówò
                        Ответить
                      • за миллиборманд лет до конца сайта
                        Ответить
        • Я тоже. Ничего зазорного тут нету.
          Ответить
    • > Возвращение назад после провала, и так до бесконечности....
      EINTR, кстати, примерно так и обрабатывают.
      Ответить

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