To pytanie jest w rzeczywistości wynikiem interesującej dyskusji, która odbyła się jakiś czas temu na stronieming.reddit.com. Zasadniczo sprowadza się do następującego kodu:
int foo(int bar)
{
int return_value = 0;
if (!do_something( bar )) {
goto error_1;
}
if (!init_stuff( bar )) {
goto error_2;
}
if (!prepare_stuff( bar )) {
goto error_3;
}
return_value = do_the_thing( bar );
error_3:
cleanup_3();
error_2:
cleanup_2();
error_1:
cleanup_1();
return return_value;
}
Użycie goto
tutaj wydaje się być najlepszą drogą, skutkując najczystszym i najbardziej wydajnym kodem ze wszystkich możliwości, a przynajmniej tak mi się wydaje. Cytując Steve'a McConnell'a w Code Complete :
Goto jest przydatny w procedurze, która przydziela zasoby, wykonuje operacje na tych zasobach, a następnie zwalnia zasoby. Za pomocą goto możesz wyczyścić jedną sekcję kodu. Goto zmniejsza prawdopodobieństwo, że zapomnisz o zwolnieniu zasobów w każdym miejscu, w którym wykryjesz błąd.
Inne wsparcie dla tego podejścia pochodzi z książki Linux Device Drivers w tej sekcji .
Co myślisz? Czy ten przypadek jest prawidłowy dla goto
w C? Czy wolałbyś inne metody, które generują bardziej zawiły i / lub mniej wydajny kod, ale unikają goto
?
źródło
on error goto error
system obsługi błędów :)Odpowiedzi:
FWIF, uważam, że idiom obsługi błędów, który podałeś w przykładzie pytania, jest bardziej czytelny i łatwiejszy do zrozumienia niż jakiekolwiek alternatywy podane w dotychczasowych odpowiedziach. Chociaż
goto
jest to ogólnie zły pomysł, może być przydatny do obsługi błędów, gdy jest wykonywany w prosty i jednolity sposób. W tej sytuacji, mimo że jest to agoto
, jest używany w dobrze zdefiniowany i mniej lub bardziej uporządkowany sposób.źródło
Zasadniczo unikanie goto jest dobrym pomysłem, ale nadużycia, które były powszechne, gdy Dijkstra po raz pierwszy napisał „GOTO uważane za szkodliwe”, nie przychodzą obecnie nawet do głowy większości ludzi jako opcja.
To, co opisujesz, jest uogólnionym rozwiązaniem problemu obsługi błędów - jest w porządku, o ile jest ostrożnie używane.
Twój konkretny przykład można uprościć w następujący sposób (krok 1):
int foo(int bar) { int return_value = 0; if (!do_something(bar)) { goto error_1; } if (!init_stuff(bar)) { goto error_2; } if (prepare_stuff(bar)) { return_value = do_the_thing(bar); cleanup_3(); } error_2: cleanup_2(); error_1: cleanup_1(); return return_value; }
Kontynuacja procesu:
int foo(int bar) { int return_value = 0; if (do_something(bar)) { if (init_stuff(bar)) { if (prepare_stuff(bar)) { return_value = do_the_thing(bar); cleanup_3(); } cleanup_2(); } cleanup_1(); } return return_value; }
Uważam, że jest to odpowiednik oryginalnego kodu. Wygląda to szczególnie czysto, ponieważ sam oryginalny kod był bardzo czysty i dobrze zorganizowany. Często fragmenty kodu nie są tak uporządkowane (chociaż zaakceptowałbym argument, że powinny); na przykład często jest więcej stanów do przekazania do procedur inicjalizacyjnych (konfiguracyjnych) niż pokazano, a zatem więcej stanów do przekazania także do procedur czyszczących.
źródło
Dziwię się, że nikt nie zasugerował tej alternatywy, więc nawet jeśli to pytanie trwało jakiś czas, dodam je: jednym z dobrych sposobów rozwiązania tego problemu jest użycie zmiennych do śledzenia bieżącego stanu. Jest to technika, której można użyć bez względu na to, czy
goto
jest używana do uzyskania kodu porządkującego. Jak każda technika kodowania ma zalety i wady i nie będzie pasować do każdej sytuacji, ale jeśli wybierasz styl, warto go rozważyć - zwłaszcza jeśli chcesz tego uniknąć,goto
nie kończąc na głęboko zagnieżdżonychif
s.Podstawową ideą jest to, że dla każdego działania porządkującego, które może być konieczne, istnieje zmienna, z której wartości możemy określić, czy czyszczenie wymaga wykonania, czy nie.
goto
Najpierw pokażę wersję, ponieważ jest bliżej kodu z oryginalnego pytania.int foo(int bar) { int return_value = 0; int something_done = 0; int stuff_inited = 0; int stuff_prepared = 0; /* * Prepare */ if (do_something(bar)) { something_done = 1; } else { goto cleanup; } if (init_stuff(bar)) { stuff_inited = 1; } else { goto cleanup; } if (prepare_stuff(bar)) { stufF_prepared = 1; } else { goto cleanup; } /* * Do the thing */ return_value = do_the_thing(bar); /* * Clean up */ cleanup: if (stuff_prepared) { unprepare_stuff(); } if (stuff_inited) { uninit_stuff(); } if (something_done) { undo_something(); } return return_value; }
Zaletą tego w porównaniu z niektórymi innymi technikami jest to, że jeśli zmieni się kolejność funkcji inicjalizacyjnych, poprawne czyszczenie będzie nadal miało miejsce - na przykład przy użyciu
switch
metody opisanej w innej odpowiedzi, jeśli kolejność inicjalizacji ulegnie zmianie, toswitch
musi być bardzo ostrożnie edytowany, aby uniknąć próby wyczyszczenia czegoś, co w rzeczywistości nie zostało zainicjowane.Niektórzy mogą twierdzić, że ta metoda dodaje całą masę dodatkowych zmiennych - i rzeczywiście w tym przypadku jest to prawda - ale w praktyce często istniejąca zmienna już śledzi lub może być zmuszona do śledzenia wymaganego stanu. Na przykład, jeśli
prepare_stuff()
faktycznie jest wywołaniem domalloc()
lub doopen()
, wówczas można użyć zmiennej przechowującej zwrócony wskaźnik lub deskryptor pliku - na przykład:int fd = -1; .... fd = open(...); if (fd == -1) { goto cleanup; } ... cleanup: if (fd != -1) { close(fd); }
Teraz, jeśli dodatkowo śledzimy stan błędu za pomocą zmiennej, możemy
goto
całkowicie uniknąć i nadal poprawnie wyczyścić, bez wcięć, które stają się coraz głębsze, im więcej potrzebujemy inicjalizacji:int foo(int bar) { int return_value = 0; int something_done = 0; int stuff_inited = 0; int stuff_prepared = 0; int oksofar = 1; /* * Prepare */ if (oksofar) { /* NB This "if" statement is optional (it always executes) but included for consistency */ if (do_something(bar)) { something_done = 1; } else { oksofar = 0; } } if (oksofar) { if (init_stuff(bar)) { stuff_inited = 1; } else { oksofar = 0; } } if (oksofar) { if (prepare_stuff(bar)) { stuff_prepared = 1; } else { oksofar = 0; } } /* * Do the thing */ if (oksofar) { return_value = do_the_thing(bar); } /* * Clean up */ if (stuff_prepared) { unprepare_stuff(); } if (stuff_inited) { uninit_stuff(); } if (something_done) { undo_something(); } return return_value; }
Ponownie istnieje potencjalna krytyka tego:
if (oksofar)
sprawdzeń do pojedynczego skoku do kodu czyszczącego (z pewnością tak robi GCC) - aw każdym razie przypadek błędu jest zwykle mniej krytyczny dla wydajności.Czy to nie dodaje kolejnej zmiennej? W tym przypadku tak, ale często
return_value
zmienna może być używana do odgrywania roli, któraoksofar
tutaj gra. Jeśli skonstruujesz swoje funkcje tak, aby zwracały błędy w spójny sposób, możesz nawet uniknąć drugiegoif
w każdym przypadku:int return_value = 0; if (!return_value) { return_value = do_something(bar); } if (!return_value) { return_value = init_stuff(bar); } if (!return_value) { return_value = prepare_stuff(bar); }
Jedną z zalet takiego kodowania jest to, że spójność oznacza, że każde miejsce, w którym pierwotny programista zapomniał sprawdzić zwracaną wartość, wystaje jak bolący kciuk, co znacznie ułatwia znajdowanie (tej jednej klasy) błędów.
A więc - to (jeszcze) jeszcze jeden styl, który można wykorzystać do rozwiązania tego problemu. Użyty poprawnie pozwala na bardzo czysty, spójny kod - i jak każda technika, w niepowołanych rękach może skończyć się tworzeniem kodu, który jest rozwlekły i zagmatwany :-)
źródło
Problem ze
goto
słowem kluczowym jest w większości źle zrozumiany. To nie jest po prostu złe. Musisz tylko zdawać sobie sprawę z dodatkowych ścieżek sterowania, które tworzysz przy każdym goto. Trudno jest uzasadnić swój kod, a tym samym jego ważność.FWIW, jeśli zajrzysz do samouczków developer.apple.com, przyjmą oni podejście goto do obsługi błędów.
Nie używamy goto. Większe znaczenie ma zwracane wartości. Obsługa wyjątków odbywa się za pomocą
setjmp/longjmp
- cokolwiek możesz.źródło
Nie ma nic moralnie złego w stwierdzeniu goto, tak jak nie jest coś moralnie nie tak ze wskazówkami (void) *.
Wszystko zależy od tego, jak używasz narzędzia. W przedstawionym przez ciebie (trywialnym) przypadku instrukcja case może osiągnąć tę samą logikę, aczkolwiek z większym narzutem. Prawdziwe pytanie brzmi: „jakie jest moje wymaganie dotyczące prędkości?”
goto jest po prostu szybki, zwłaszcza jeśli uważasz, aby kompilować się do krótkiego skoku. Idealny do zastosowań, w których liczy się szybkość. W przypadku innych aplikacji prawdopodobnie sensowne jest zajęcie się problemem z przypadkiem if / else + dla łatwości konserwacji.
Pamiętaj: goto nie zabija aplikacji, programiści zabijają aplikacje.
UPDATE: Oto przykład przypadku
int foo(int bar) { int return_value = 0 ; int failure_value = 0 ; if (!do_something(bar)) { failure_value = 1; } else if (!init_stuff(bar)) { failure_value = 2; } else if (prepare_stuff(bar)) { return_value = do_the_thing(bar); cleanup_3(); } switch (failure_value) { case 2: cleanup_2(); case 1: cleanup_1(); default: break ; } }
źródło
GOTO jest przydatne. To coś, co potrafi twój procesor i dlatego powinieneś mieć do niego dostęp.
Czasami chcesz dodać coś do swojej funkcji i możesz to łatwo zrobić. Oszczędza czas.
źródło
Ogólnie rzecz biorąc, wziąłbym pod uwagę fakt, że fragment kodu można najdokładniej napisać, wykorzystując
goto
jako symptom, że przepływ programu jest prawdopodobnie bardziej skomplikowany niż jest to ogólnie pożądane. Łączenie innych struktur programu w dziwny sposób, aby uniknąć użycia,goto
byłoby próbą leczenia objawu, a nie choroby. Twój konkretny przykład może nie być zbyt trudny do wdrożenia bezgoto
:ale jeśli czyszczenie miało się odbyć tylko wtedy, gdy funkcja zawiodła,
goto
sprawę można by załatwić, umieszczającreturn
tuż przed pierwszą etykietą docelową. Powyższy kod wymagałby dodaniareturn
w linii oznaczonej*****
.W scenariuszu „porządkowanie nawet w normalnym przypadku” uznałbym użycie
goto
za jaśniejsze niż konstrukcjedo
/while(0)
, między innymi dlatego, że same etykiety docelowe praktycznie krzyczą „SPÓJRZ NA MNIE” znacznie bardziej niż konstrukcjebreak
ido
/while(0)
. W przypadku „czyszczenie tylko w przypadku błędu”return
instrukcja kończy się na tym, że musi znajdować się w najgorszym możliwym miejscu z punktu widzenia czytelności (instrukcje powrotu powinny generalnie znajdować się na początku funkcji lub na tym, jak „wygląda” koniec); posiadaniereturn
etykiety tuż przed etykietą docelową spełnia tę kwalifikację znacznie łatwiej niż posiadanie etykiety tuż przed końcem „pętli”.BTW, jeden scenariusz, w którym czasami używam
goto
do obsługi błędów, znajduje się wswitch
instrukcji, gdy kod dla wielu przypadków ma ten sam kod błędu. Chociaż mój kompilator często byłby wystarczająco inteligentny, aby rozpoznać, że wiele przypadków kończy się tym samym kodem, myślę, że jaśniej jest powiedzieć:Chociaż można by zamienić
goto
instrukcje na{handle_error(); break;}
i chociaż można by użyć pętlido
/while(0)
wraz zcontinue
przetwarzaniem opakowanego pakietu warunkowego wykonania, tak naprawdę nie sądzę, aby było to bardziej przejrzyste niż użyciegoto
. Co więcej, chociaż może być możliwe skopiowanie kodu zPACKET_ERROR
dowolnego miejsca, w którymgoto PACKET_ERROR
jest używany, i chociaż kompilator może napisać zduplikowany kod raz i zastąpić większość wystąpień skokiem do tej udostępnionej kopii, użycie narzędziagoto
ułatwia zauważenie miejsc które ustawiają pakiet trochę inaczej (np. jeśli instrukcja „wykonaj warunkowo” zdecyduje się nie wykonywać).źródło
Osobiście jestem zwolennikiem „Siła dziesięciu - 10 zasad pisania krytycznego kodu bezpieczeństwa” .
Dołączę mały fragment tego tekstu, który ilustruje, jak sądzę, dobry pomysł na temat goto.
Reguła: Ogranicz cały kod do bardzo prostych konstrukcji przepływu sterowania - nie używaj instrukcji goto, konstrukcji setjmp lub longjmp oraz rekursji bezpośredniej lub pośredniej.
Uzasadnienie: Prostszy przepływ kontroli przekłada się na lepsze możliwości weryfikacji i często skutkuje lepszą przejrzystością kodu. Wygnanie rekurencji jest tutaj chyba największą niespodzianką. Jednak bez rekurencji mamy gwarancję, że mamy graf wywołania acyklicznego funkcji, który może być wykorzystany przez analizatory kodu i może bezpośrednio pomóc w udowodnieniu, że wszystkie wykonania, które powinny być ograniczone, są w rzeczywistości ograniczone. (Należy zauważyć, że ta reguła nie wymaga, aby wszystkie funkcje miały jeden punkt powrotu - chociaż często upraszcza to również przepływ sterowania. Jest jednak wystarczająco dużo przypadków, w których wczesne zwrócenie błędu jest prostszym rozwiązaniem).
Wykluczenie używania goto wydaje się złe, ale:
Jeśli na pierwszy rzut oka zasady wydają się drakońskie, pamiętaj, że mają na celu umożliwienie sprawdzenia kodu, w którym bardzo dosłownie twoje życie może zależeć od jego poprawności: kod używany do kontrolowania samolotu, którym lecisz, elektrowni jądrowej kilka mil od miejsca, w którym mieszkasz, lub statek kosmiczny, który przenosi astronautów na orbitę. Zasady działają jak pasy bezpieczeństwa w Twoim aucie: początkowo mogą być trochę niewygodne, ale po pewnym czasie ich użycie staje się drugą naturą i niewyobrażalne staje się ich niewyobrażalne.
źródło
goto
jest użycie jakiegoś zestawu „sprytnych” wartości logicznych w głęboko zagnieżdżonych ifach lub pętlach. To naprawdę nie pomaga. Może Twoje narzędzia będą go grok lepiej, ale ty nie jesteś i będzie ważniejsze.Zgadzam się, że porządkowanie goto w odwrotnej kolejności podanej w pytaniu jest najczystszym sposobem na uporządkowanie większości funkcji. Ale chciałem też zwrócić uwagę, że czasami i tak chcesz wyczyścić swoją funkcję. W takich przypadkach używam następującego wariantu if (0) {label:} idiom, aby przejść do właściwego punktu procesu czyszczenia:
int decode ( char * path_in , char * path_out ) { FILE * in , * out ; code c ; int len ; int res = 0 ; if ( path_in == NULL ) in = stdin ; else { if ( ( in = fopen ( path_in , "r" ) ) == NULL ) goto error_open_file_in ; } if ( path_out == NULL ) out = stdout ; else { if ( ( out = fopen ( path_out , "w" ) ) == NULL ) goto error_open_file_out ; } if( read_code ( in , & c , & longueur ) ) goto error_code_construction ; if ( decode_h ( in , c , out , longueur ) ) goto error_decode ; if ( 0 ) { error_decode: res = 1 ;} free_code ( c ) ; if ( 0 ) { error_code_construction: res = 1 ; } if ( out != stdout ) fclose ( stdout ) ; if ( 0 ) { error_open_file_out: res = 1 ; } if ( in != stdin ) fclose ( in ) ; if ( 0 ) { error_open_file_in: res = 1 ; } return res ; }
źródło
Wydaje mi się, że
cleanup_3
powinien to zrobić, a potem zadzwońcleanup_2
. Podobnie,cleanup_2
należy wykonać czyszczenie, a następnie wywołać cleanup_1. Wygląda na to, że za każdym razem, gdy to zrobiszcleanup_[n]
,cleanup_[n-1]
jest to wymagane, dlatego powinna być odpowiedzialna za metodę (aby na przykładcleanup_3
nigdy nie można było wywołać bez wywołaniacleanup_2
i prawdopodobnie powodując wyciek).Biorąc pod uwagę takie podejście, zamiast gotos, po prostu wywołałbyś procedurę czyszczenia, a następnie wrócił.
To
goto
podejście nie jest złe ani złe , warto jednak zauważyć, że niekoniecznie jest to „najczystsze” podejście (IMHO).Jeśli szukasz optymalnej wydajności to przypuszczam, że
goto
rozwiązanie jest najlepsze. Oczekuję jednak, że będzie to istotne tylko w kilku wybranych aplikacjach krytycznych dla wydajności (np. Sterowniki urządzeń, urządzenia wbudowane itp.). W przeciwnym razie jest to mikro-optymalizacja, która ma niższy priorytet niż przejrzystość kodu.źródło
Myślę, że to pytanie jest błędne w stosunku do podanego kodu.
Rozważać:
Dlatego: do_something (), init_stuff () i Preparat_stuff () powinny wykonywać swoje własne porządki . Posiadanie oddzielnej funkcji cleanup_1 (), która czyści po do_something (), łamie filozofię hermetyzacji. To zły projekt.
Jeśli zrobili własne czyszczenie, foo () staje się całkiem proste.
Z drugiej strony. Jeśli foo () faktycznie stworzyło swój własny stan, który należało zburzyć, to goto byłoby odpowiednie.
źródło
Oto, co wolałem:
bool do_something(void **ptr1, void **ptr2) { if (!ptr1 || !ptr2) { err("Missing arguments"); return false; } bool ret = false; //Pointers must be initialized as NULL void *some_pointer = NULL, *another_pointer = NULL; if (allocate_some_stuff(&some_pointer) != STUFF_OK) { err("allocate_some_stuff step1 failed, abort"); goto out; } if (allocate_some_stuff(&another_pointer) != STUFF_OK) { err("allocate_some_stuff step 2 failed, abort"); goto out; } void *some_temporary_malloc = malloc(1000); //Do something with the data here info("do_something OK"); ret = true; // Assign outputs only on success so we don't end up with // dangling pointers *ptr1 = some_pointer; *ptr2 = another_pointer; out: if (!ret) { //We are returning an error, clean up everything //deallocate_some_stuff is a NO-OP if pointer is NULL deallocate_some_stuff(some_pointer); deallocate_some_stuff(another_pointer); } //this needs to be freed every time free(some_temporary_malloc); return ret; }
źródło
Stara dyskusja jednak… co powiesz na użycie „strzałki anty-wzorca” i późniejsze hermetyzowanie każdego zagnieżdżonego poziomu w statycznej funkcji wbudowanej? Kod wygląda na czysty, jest optymalny (gdy optymalizacje są włączone) i nie jest używane goto. Krótko mówiąc, dziel i zwyciężaj. Poniżej przykład:
static inline int foo_2(int bar) { int return_value = 0; if ( prepare_stuff( bar ) ) { return_value = do_the_thing( bar ); } cleanup_3(); return return_value; } static inline int foo_1(int bar) { int return_value = 0; if ( init_stuff( bar ) ) { return_value = foo_2(bar); } cleanup_2(); return return_value; } int foo(int bar) { int return_value = 0; if (do_something(bar)) { return_value = foo_1(bar); } cleanup_1(); return return_value; }
Jeśli chodzi o przestrzeń, tworzymy trzykrotną zmienną na stosie, co nie jest dobre, ale to znika podczas kompilacji z -O2, usuwając zmienną ze stosu i używając rejestru w tym prostym przykładzie. To, co dostałem z powyższego bloku,
gcc -S -O2 test.c
to poniższe:.section __TEXT,__text,regular,pure_instructions .macosx_version_min 10, 13 .globl _foo ## -- Begin function foo .p2align 4, 0x90 _foo: ## @foo .cfi_startproc ## %bb.0: pushq %rbp .cfi_def_cfa_offset 16 .cfi_offset %rbp, -16 movq %rsp, %rbp .cfi_def_cfa_register %rbp pushq %r14 pushq %rbx .cfi_offset %rbx, -32 .cfi_offset %r14, -24 movl %edi, %ebx xorl %r14d, %r14d xorl %eax, %eax callq _do_something testl %eax, %eax je LBB0_6 ## %bb.1: xorl %r14d, %r14d xorl %eax, %eax movl %ebx, %edi callq _init_stuff testl %eax, %eax je LBB0_5 ## %bb.2: xorl %r14d, %r14d xorl %eax, %eax movl %ebx, %edi callq _prepare_stuff testl %eax, %eax je LBB0_4 ## %bb.3: xorl %eax, %eax movl %ebx, %edi callq _do_the_thing movl %eax, %r14d LBB0_4: xorl %eax, %eax callq _cleanup_3 LBB0_5: xorl %eax, %eax callq _cleanup_2 LBB0_6: xorl %eax, %eax callq _cleanup_1 movl %r14d, %eax popq %rbx popq %r14 popq %rbp retq .cfi_endproc ## -- End function .subsections_via_symbols
źródło
Wolę używać techniki opisanej w poniższym przykładzie ...
struct lnode *insert(char *data, int len, struct lnode *list) { struct lnode *p, *q; uint8_t good; struct { uint8_t alloc_node : 1; uint8_t alloc_str : 1; } cleanup = { 0, 0 }; // allocate node. p = (struct lnode *)malloc(sizeof(struct lnode)); good = cleanup.alloc_node = (p != NULL); // good? then allocate str if (good) { p->str = (char *)malloc(sizeof(char)*len); good = cleanup.alloc_str = (p->str != NULL); } // good? copy data if(good) { memcpy ( p->str, data, len ); } // still good? insert in list if(good) { if(NULL == list) { p->next = NULL; list = p; } else { q = list; while(q->next != NULL && good) { // duplicate found--not good good = (strcmp(q->str,p->str) != 0); q = q->next; } if (good) { p->next = q->next; q->next = p; } } } // not-good? cleanup. if(!good) { if(cleanup.alloc_str) free(p->str); if(cleanup.alloc_node) free(p); } // good? return list or else return NULL return (good? list: NULL);
}
źródło: http://blog.staila.com/?p=114
źródło
Używamy
Daynix CSteps
biblioteki jako innego rozwiązania „ problemu goto ” w funkcjach init.Zobacz tutaj i tutaj .
źródło
goto