Error handling - jak to ma się do clean code'u?

Cześć wszystkim

Obsługa błędów w C jest raczej schematyczny -> tzn. wyrażenie if + ewentualne wyjście z programu. Za każdym razem jak implementuję taką obsługę błędów robię to raczej z grymasem na twarzy aniżeli cieszę się, że program wykryje wszystkie błędy i odpowiednio na nie zareaguje. Dlaczego? Dlatego, że po zaimplementowaniu obsługi błędów powstaje coś takiego na przykład:

static void server_listening_init(int *listen_socket, struct conf *config, struct sockaddr_in *server_addr) {
	
	*listen_socket = socket(AF_INET, SOCK_STREAM, 0);
	if ((*listen_socket) < 0) {
		print_error_and_exit("Creating listening socket");
	}

	FILE *config_file = open_file("ports.conf", CONF_DIR, strlen("ports.conf"), strlen(CONF_DIR));
	if (config_file == NULL) {
		print_error_and_exit("Opening config file");
	}

	fset_conf_state(config_file, config, "PORT:");
	if (!port_is_valid(config)) {
		fprintf(stderr, "%s\n", "Listening port is not valid. Try to use port from 1024 to 49151 (user ports).");
		exit(EXIT_FAILURE);
	}

	init_address_struct(server_addr, config);

	#ifdef DEBUG
		int optval = 1;
		if (setsockopt(*listen_socket, SOL_SOCKET, (SO_REUSEADDR | SO_REUSEPORT), &optval, sizeof(optval)) < 0) {
			print_error_and_exit("Listening socket option");
		}
	#endif

	int binding_error = bind(*listen_socket, (struct sockaddr *)server_addr, sizeof(*server_addr));
	if (binding_error) {
		print_error_and_exit("Binding listening socket");
	}

	if (listen(*listen_socket, 5) < 0) {
		print_error_and_exit("Listen socket");
	}

	fclose(config_file);
}

Chodzi o to, że kod funkcji bardzo się wydłuża przez te if'y łapiące błędy. Jest inna opcja, żeby te błędy obsługiwać czy muszę tak to zostawić?

Tak, bo to co pokazałeś jest błędne (nie zamykasz plików i nie zwalniasz socketów w przypadku błędu, chyba że zawsze chcesz zakończyć program w takim wypadku. Jeśli zawsze chcesz wyjść z programu, to możesz zwyczajnie rozbić to na dodatkowe funkcje, jeśli chcesz mieć fallback, to zostaje Ci goto.

Możesz też użyć jakiejś implementacji, która działa podobnie do defer z GoLang: https://github.com/trws/libdefer

Przykładowe użycie:

#include <defer.h>
DEFER_SCOPED(int64_t, copy_file, (const char *, dst_name, const char, *src_name)) {
    FILE *dst = fopen(dst_name, "w");
    if (!dst) {
        return -1;
    }
    defer(fclose, dst);
    FILE *src = fopen(src_name, "r");
    if (!src) {
        return -1;
    }
    defer(fclose, src);
    
    return copy_between_streams(dst, src);
}

Jeśli masz możliwość użycia C++ to da się to zrobić nawet jeszcze ładniej (dzięki automatycznie wykonywanym destruktorom).

Oczywiście to wciąż nie rozwiązuje twojego problemu z tym, że obsługa błędów jest w tym samym jednym miejscu.
Rozwiązaniem tego mogłoby by być zdefiniowanie własnych funkcji, które by rzucały odpowiednie błędy (np. zamiast wołania socket byś wołał własną funkcję create_socket, która by przyjmowała takie same parametry, by zwracała taki sam typ, i ona by wołała socket ale zarazem obsługiwała by błąd.

Pewnie czytałeś o SOLID i pierszy punkt "Single Responsibility Principle".
Przeczytać to jedno, zrozumieć i umieć stosować to zupełnie coś innego.
Wepchnąłeś za dużo do tej funkcji.
Powinieneś mieć osobne funkcje na:

• wczytanie konfiguracji
• walidacji konfiguracji
• oczekiwania połączania z klientem.

Zwróć uwagę, że kod obsługujący się nie obchodzi skąd się wzięła konfiguracja i na odwór kod wczytujący konfigurację ma gdzieś co się z nią dzieje potem.
Tak samo twój kod nie powinien kończyć aplikacji od razu przy każdym błędzie.
Może twoja logika biznesowa będzie obsługiwać błędy? Jeśli będziesz kończenie aplikacji na każdy błąd będzie u ciebie regułą, to potem będziesz miał problemy by informacja o błędzie dotarła tam, gdzie mogła by być obsłużona.

int config_load_from_file(struct conf *config, FILE *f)
{
    fset_conf_state(f, config, "PORT:");
    if (!port_is_valid(config)) {
        fprintf(stderr, "%s\n", "Listening port is not valid. Try to use port from 1024 to 49151 (user ports).");
        return ERROR_INVALID_CONFIG;
    }
    return SUCCESS;
}

int config_load_from_file_named(struct conf *config, const char* fileName)
{
    FILE *file = open_file(fileName, CONF_DIR, strlen(fileName), strlen(CONF_DIR));
    if (file == NULL) {
        fprintf(stderr , "Opening config file: %s", fileName);
        return ERROR_OPENING_FILE;
    }
    int result =  config_load_from_file(config, file);
    close(file); // odpowiadające sobie open / close - zawsze powinny być w obrębie jednego spojrzenia - np 1/3 ekranu
    return result;
}

int config_get_XXXX_adress(struct conf *config, struct sockaddr_in *server_addr)
{
    init_address_struct(server_addr, config);
}

int server_listening_init(int *listen_socket, struct sockaddr *server_addr) {
    *listen_socket = socket(AF_INET, SOCK_STREAM, 0);
    if ((*listen_socket) < 0) {
        perror("Creating listening socket");
        return ERROR_CREATING_SOCKET;
    }

    #ifdef DEBUG
        int optval = 1;
        if (setsockopt(*listen_socket, SOL_SOCKET, (SO_REUSEADDR | SO_REUSEPORT), &optval, sizeof(optval)) < 0) {
            print_error_and_exit("Listening socket option");
        }
    #endif

    int binding_error = bind(*listen_socket, server_addr, sizeof(*server_addr));
    if (binding_error) {
        perror("Binding listening socket");
        return ERROR_BIND_FAILURE;
    }

    if (listen(*listen_socket, 5) < 0) {
        perror("Listen socket");
        return ERROR_LISTEN_FAILURE;
    }
    return SUCCESS;
}

Nie napisałem funkcji łączącej tego do kupy, bo to są rzeczy, które powinny być od siebie dość odległe.
Jak widać trzymanie rzeczy osobno, czyni obsługę błędów mniej upierdliwą.
Tak samo lista argumentów server_listening_init zrobiła się sensowniejsza i krótsza.

Disclaimer: kod przerobiłem na szybko, by pokazać filozofię. Pewnie są błędy i dziwne niedoróbki. Pewnie jeszcze bardzie da się zmniejszyć ta funkcję.

Jeśli będziesz kończenie aplikacji na każdy błąd będzie u ciebie regułą, to potem będziesz miał problemy by informacja o błędzie dotarła tam, gdzie mogła by być obsłużona.

Nie mam zamiaru kończyć działania aplikacji po każdym błędzie. Jestem w trakcie pracy nad obsługą logów, w których każda informacja o błędzie będzie dostępna, jeśli użytkownik będzie tego chciał i ogarnie sobie config. Po prostu bindowanie nasłuchującego socketu i wszystkie te pracy, które dzieją się na poziomie w ogóle inicjalizacji serwera są obsługiwane tak, aby program kończył działanie.

Nie napisałem funkcji łączącej tego do kupy, bo to są rzeczy, które powinny być od siebie dość odległe.

Skoro te funkcjonalności mają być od siebie odległe to znaczy, że nie mogę wykonać funkcji pobierającej wartość portu nasłuchującego z pliku konfiguracyjnego bezpośrednio wewnątrz funkcji init_server? W takim razie bardzo to utrudnia sprawę. Na pewno kodu inicjującego serwer nie interesuje kod pobierający wartości odpowiednich właściwości z plików konfiguracyjnych? Jak w takim wypadku odbywa się czytanie z konfiguracji i reakcja na dane wewnątrz tego typu programów?