Jeśli obsługiwać wszystkie błędy mogące wystąpić z wywołania funkcji systemowych, to powstaje szereg bloków zawierających te same instrukcje tylko z dodawanymi kolejnymi na początku, tak jak w przykładzie tutaj.
Kompilator “gcc” ma opcję “-ftree-tail-merge”, która włącza optymalizację takich bloków; o ile — jak się domyślam — zapis funkcji w tych blokach jest w tej samej kolejności, z czym może być problem w dłuższym programie. Natomiast taki zapis szeregów bloków nie jest wygodny.
Jest jakiś przykład obsługi błędów, w którym wywołania takich funkcji zwalniających zasoby nie muszą się powtarzać? A może jakoś inaczej obsługiwać błędy?
0
3
Możesz funkcję zwalniania dać na końcu tak jak przy wychodzeniu z funkcji, a przy błędach skakać za pomocą goto do odpowiedniego fragmentu do zwalniania, tak się czasem robi.
Lepiej to użyć kompilatora C++ do języka C i teraz możesz do struktur dodawać destruktory, które za ciebie zwolnią zasoby gdy opuszczą scope funkcji.
A jeszcze lepiej to przesiąść się na C++ jeśli nic cię nie powstrzymuje, chyba że w kernelu linuxa piszesz...
3
overcq napisał(a):
tak jak w przykładzie tutaj.
Każdy wyznawca C powie, że C jest zajedwabiste i najlepsze (dla mnie w/w fragment to ohyda)
Wypłynięcie C na rolę hegemona, choć w latach 1970tych były lepsze, bezpieczniejsze języki, to dla mnie bilion dollar bug
Jest jakiś przykład obsługi błędów, w którym wywołania takich funkcji zwalniających zasoby nie muszą się powtarzać? A może jakoś inaczej obsługiwać błędy?
Tak jak wskazał @Wypierdzistyy , tylko i wyłącznie C++
Wypierdzistyy napisał(a):
Możesz funkcję zwalniania dać na końcu tak jak przy wychodzeniu z funkcji, a przy błędach skakać za pomocą goto do odpowiedniego fragmentu do zwalniania, tak się czasem robi.
Mi w gołym C nigdy sie nie udawało upilnować wszystkich niepowodzeń, postawić "odpowiedniej ilości goto" itd
Podatnosc na błedy jest kolosalna (np zapomnisz zainicjować handler na zero/null, i pozornie ma senswoną wartość, podlegajacą zwolnieniu)
Lepiej to użyć kompilatora C++ do języka C i teraz możesz do struktur dodawać destruktory, które za ciebie zwolnią zasoby gdy opuszczą scope funkcji.
A jeszcze lepiej to przesiąść się na C++ jeśli nic cię nie powstrzymuje, chyba że w kernelu linuxa piszesz...
+1
Destruktor, ale również gwaracja wszelkiej "prywatyzacji" czy enkapsulacji
Taka C++ warstwa nad surowym Win API jak MFC ma handlery do zasobów jako klasy, przy czym uzywa "przez wartość", i całkiem dobrze to działa (co nie znaczy, ze polecam środowisko)
A propos WIn API, jest przynajmniej jedna funkcja, która zawsze zwraca TRUE niezależnie od powodzenia, jest udokumentowane ale nie jest to zmieniane przez kompatybilność
Warto spojrzeć na smart pointery, i czymś w tym stylo obskoczyć zasoby
3
IMO twój kod podpada pod:
Linus Torvalds: kernel.org coding-style
Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program.
Generalnie ta funkcja E_main_I_build jest a dług przez co jest nie od ogarnięcia. Na dodatek jej nazwa nic nie mówi, więc nawet nie mam punktu zaczepienia jak ją poprawiać.
Jak pisuje w C to dzielę sobie funkcję w ten sposób:
struct MyData {
....
};
void MyStructInit(struct MyData* data)
{
....
}
int MyStructLoadFromStream(struct MyData* data, FILE* f)
{
// regular logic
}
int MyStructLoadFromFileNamed(struct MyData* data, const char* name)
{
int result = 0;
FILE* f = fopen(name, "r");
if (f) {
result= MyStructLoadFromStream(data, f);
fclose(f);
}
return result;
}
Jednak zwykle dłubie w C++ i używam API C, to opakowuje je sobie wzorcem RAII (kiedyś dawałem tu przykład na OpenSSL - gdzieś był lepszy bardziej kompletny przykład).
4
Początek sprzątania tego kodu:
int MyForEachFindFiles(const char* pattern, int (*f)(WIN32_FIND_DATA *found_dir, void* context), void* context)
{
WIN32_FIND_DATA found_dir;
int result = 0;
HANDLE h_find_dir = FindFirstFile( pattern, &found_dir );
if( h_find_dir != INVALID_HANDLE_VALUE )
{
do {
result = f(&found_dir, context);
} while(result && FindNextFile( h_find_dir, &found_dir ))
FindClose( h_find_dir );
}
return result;
}
struct EMainBuildResources {
pcre2_code *struct_union_regex;
pcre2_code *preproc_if_regex;
pcre2_code *h_xyi_regex_b;
pcre2_match_data *match_data;
pcre2_code *enum_struct_union_regex_3;
pcre2_code *def_name_asterisk_regex;
pcre2_code *def_name_regex;
pcre2_code *d_regex;
pcre2_code *e_regex;
};
pcre2_code *_EBuiltREAndReprotError(const char *re)
{
pcre2_code * r;
int error_code;
PCRE2_SIZE error_offset;
r = pcre2_compile( pattern, PCRE2_ZERO_TERMINATED, 0, &error_code, &error_offset, 0 );
if (!r)
{
char libMsg[256];
pcre2_get_error_message(error_code, libMsg, sizeof(libMsg));
fprintf(stderr, "%s at pos %zd", libMsg, error_offset);
}
return r;
}
int EMainBuildResourcesInit(struct EMainBuildResources *r)
{
memset(r, 0, sizeof(*r));
if( !(r->struct_union_regex = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->preproc_if_regex = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->h_xyi_regex_b = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->match_data = pcre2_match_data_create( 3, 0 )) )
{
fprintf(stderr, "Unable to allocate match data.");
return 0;
}
if( !(r->enum_struct_union_regex_3 = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->def_name_asterisk_regex = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->def_name_regex = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->d_regex = _EBuiltREAndReprotError("")) ) return 0;
if( !(r->e_regex = _EBuiltREAndReprotError("")) ) return 0;
return 1;
}
void EMainBuildResourcesDeinit(struct EMainBuildResources *r)
{
// pcre2_code_free and pcre2_match_data_free does nothing for NULL
pcre2_code_free( r->struct_union_regex );
pcre2_code_free( r->preproc_if_regex );
pcre2_code_free( r->h_xyi_regex_b );
pcre2_match_data_free( r->match_data );
pcre2_code_free( r->enum_struct_union_regex_3 );
pcre2_code_free( r->def_name_asterisk_regex );
pcre2_code_free( r->def_name_regex );
pcre2_code_free( r->d_regex );
pcre2_code_free( r->e_regex );
}
int EMainBuildResourcesProcessFile(WIN32_FIND_DATA *found_dir, struct EMainBuildResources *r)
{
......
}
int E_main_I_build(void)
{
int result = 0;
struct EMainBuildResources data;
EMainBuildResourcesInit(&data);
result = MyForEachFindFiles("..\\module\\*", EMainBuildResourcesProcessFile, &data);
EMainBuildResourcesDeinit(&data);
return result;
}
0
@MarekR22: Dzięki za przykład. Popróbuję coś w tę stronę. Największy problem miałem właśnie z utworzeniem struktury na dane czyli kontekstu do FindFile. Mam złe nawyki z możliwości definicji zmiennej wewnątrz kodu.
Czyli proponujesz tutaj zwalniać wszystko zawsze tylko korzystać ze sprawdzania, czy zostało zainicjowane, bez dopasowania zwalniania do logiki wewnątrz programu. Z wymienionymi zasobami to jest to dość proste, ponieważ są i tak inicjowane na zewnątrz. Będę szukał, co zrobić z zasobami wewnątrz pętli, i może to jakoś ogarnę. A po zmianach porównam, jak sobie kompilator poradzi z tym kodem.
2
Chodzi o to aby nie kopiować kodu zwalniania zasobów ( free close ) przy błedzie w funkcji?
Goto oczywiście. Niektórzy pewnie dostaną na samo to słowo sraczki ale naprawdę nic specjalnego.
Funkcje systemowe też czasem są tak napisane.
Ewentualnie możesz napisać do tego specjalną funkcję błędu ( hint może brać NULL jak danego zasobu nie trzeba zwalniać ) albo może jakieś macro ale to już luźna myśl.
0
ksh napisał(a):
Goto oczywiście.
Zaproszenie do flame'a? po 54 latach temat chyba powinien być zamknięty.
0
Nawet nie chciało mi się tego czytać. Elaborat dziadu napisał, pewnie case też nie używa. 54 lata temu ktoś się wygłupił. Temat chyba powinien być zamknięty.
Tak się składa, że problem poruszony w wątku jest jednym z nielicznych przypadków gdy goto się sprawdza i jest dosyć dobrze czytelne.
Wszystkie gota prowadzą na koniec funkcji za return 0; .
Kod taki się spotyka i jest, i czytelny, i dobry.
A jeszcze mi się przypomniało. Czasami też tak robiłem.
Wsadzam wszystko w pętlę
I robię break na błedzie.
Na końcu pętli return sukces.
voila.
0
O kurczę, goto :D
@overcq jak widać w tym kodzie, alokowanie zasobów jest łańcuchowe.
Więc jak jest łańcuchowe i może się wysypać w każdym miejscu łańcucha to należałoby użyć C++ lub coś sensownego zdrutować (RAII).
Ze swojej strony podam coś takiego na listach:
struct Resources{
struct list_resources_type_a *res_a_list;
struct list_resources_type_b *res_b_list;
};
void add_resource_a_to_list(type_res_a *res_a){
//dodaj zasób do listy ze struktury Resources
...
}
//później gdy coś pójdzie nie tak:
void clear_all_resources(){
//wyczyść wszystkie zasoby z list ze struktury Resources które zostały dodane do tej pory
...
}
Z czegoś tego typu, korzystałem na ARMach, aby utworzyć pewne STL'e w C, ponieważ C++ dodawał za dużo kodu z allocatorami.
Lekko porządkuje sprawę i trudniej o błąd, ale jednak jest to dziwna proteza.
2
Wypierdzistyy napisał(a):
Możesz funkcję zwalniania dać na końcu tak jak przy wychodzeniu z funkcji, a przy błędach skakać za pomocą goto do odpowiedniego fragmentu do zwalniania, tak się czasem robi.
ksh napisał(a):
Chodzi o to aby nie kopiować kodu zwalniania zasobów ( free close ) przy błedzie w funkcji?
Goto oczywiście.
To mile ze sa jeszcze programisci ktorzy sa w stanie wybrac najprostsze i najczytelniesze rozwiazanie zamiast kombinowac z rozwiazaniami na poziomie "drapanie sie w prawe ucho lewa noga przy pomocy grabi" dlatego ze "nie uzywamy goto z powodu bo nie".
Potwierdzam: jak pisalem w C to goto byla jedna z najczesciej uzywanych instrukcji, i byla uzywana wlasnie do zwalniania zasobow.
0
eleventeen napisał(a):
Wypierdzistyy napisał(a):
Możesz funkcję zwalniania dać na końcu tak jak przy wychodzeniu z funkcji, a przy błędach skakać za pomocą goto do odpowiedniego fragmentu do zwalniania, tak się czasem robi.
ksh napisał(a):
Chodzi o to aby nie kopiować kodu zwalniania zasobów ( free close ) przy błedzie w funkcji?
Goto oczywiście.
To mile ze sa jeszcze programisci ktorzy sa w stanie wybrac najprostsze i najczytelniesze rozwiazanie zamiast kombinowac z rozwiazaniami na poziomie "drapanie sie w prawe ucho lewa noga przy pomocy grabi" dlatego ze "nie uzywamy goto z powodu bo nie".
Potwierdzam: jak pisalem w C togotobyla jedna z najczesciej uzywanych instrukcji, i byla uzywana wlasnie do zwalniania zasobow.
Każda zasada / wzorzec / "wzorzec" / sugestia, użyteczna w połączeniu z rozsądkiem, gdy przejdzie z tego właśnie etapu na etap religijny staje się wynaturzeniem.
Nie tylko w programowaniu, w szerokiej budowlance głupie i pobieżne pomiary (dupochrnony), politycznie poprawne przejścia dla pieszych co 50m itd...
W naszej branży to po pierwsze małpi zachwyt, hypen driven development ("ciągle jesteśmy barbarzyńcami, których zachwycają nowe szkiełka", cyt z głowy Saint Exupery), paradoksalnie elita pod wzgledem IQ *) ma ogromny odruch stadny jak przekupki na placu ... po "jak nie będę używał X/będę używał Y, to mnie nie zarekrutują"
*) przynajmniej chciałbym w to wierzyć
0
Instrukcja goto, używanie jej w celu obsługi błędów nie jest wcale złe. Ewentualnie, jeśli chodzi o zwolnienie zasobów całego programu to użyj atexit.
#include <stdlib.h>
int *tablica;
void cleanup ()
{ /* funkcja zwalniająca zasoby */
if (tablica)
free (tablica);
}
int main ()
{ atexit (cleanup);
tablica = calloc (sizeof (int), 100);
exit (0);
}
0
Manna5 napisał(a):
Instrukcja
goto, używanie jej w celu obsługi błędów nie jest wcale złe. Ewentualnie, jeśli chodzi o zwolnienie zasobów całego programu to użyjatexit.
Rzecz w tym, że nawet najbardziej pobożnie użyta, zajmuje się tylko jednym z aspektów gospdarki zasobami. Np goto nie ma nic do gwarantowanej poprawnej inicjacji.
2
No właśnie, czemu nie skorzystać z rozszerzenia GCC?
void free_x(X** x);
void free_y(Y** y);
void free_z(Z** z);
Z* funkcja() {
X* __attribute__((cleanup(free_x))) x = make_x();
if (!x) return NULL;
Y* __attribute__((cleanup(free_y))) y = make_y(x);
if (!y) return NULL;
Z* __attribute__((cleanup(free_z))) z = make_z(y);
if (!z) return NULL;
return z;
}
0
Wersja zmieniona do funkcji, jak zaproponował @MarekR22, kompiluje się do trochę większego pliku wykonywalnego, ale jest łatwiejsza w utrzymaniu. Dla wewnętrznych zasobów nadal używam kilka goto. Czyli jest to wersja mieszana. Funkcje nadal są długie, ale już jest czyściej. Zostawić tę wersję?
0
Slabe to jest:
Err_4:
CloseHandle( h_file );
Err_3:
HeapFree( E_main_S_process_heap, 0, enc );
Err_2:
CloseHandle( h_file_cx );
Err_1:
HeapFree( E_main_S_process_heap, 0, file );
Nie jest trudno doprowadzic do wyciekow jak sie nie zgadnie gdzie skoczyc, albo w przyszlosci cokolwiek sie w kodzie zmieni.
Przyklad jak to lata temu moglo wygladac w javie (bedzie nie calkiem poprawnie ale pewnie malo kto zauwazy wiec bede udawal ze jest OK) i jak w miare bezposrednio mozna przetlumaczyc na C:
Obj obj1 = null; | Obj obj1 = NULL;
Obj obj2 = null; | Obj obj2 = NULL;
Obj obj3 = null; | Obj obj3 = NULL;
try { |
obj1 = new ... | obj1 = ...
| if (obj1 == NULL) {
| goto finally;
| }
obj2 = new ... | obj2 = ...
| if (obj2 == NULL) {
| goto finally;
| }
obj3 = new ... | obj3 = ...
| if (obj3 == NULL) {
| goto finally;
| }
... | ...
if (...) { | if (...) {
return ... | result = ...
| goto finally;
} | }
... | ...
} finally { | finally:
if (obj3 != null) { |
obj3.close(); | close(obj3);
} |
if (obj2 != null) { |
obj2.close(); | close(obj2);
} |
if (obj1 != null) { |
obj1.close(); | close(obj1);
} |
} |
| return result;
A z tym goto Cont_2; to juz przesadziles. Czytelnosc tego jest zadna. A co do poprawnosci to nie mam pojecia - ciagle zwalnianie zasobow jest w wielu miejscach i analizowanie tego jest nieprzyjemne.