[C] NetCom - prośba o sprawdzenie programu

[dawwin]

Niedawno zacząłem swoją przygodę z programowaniem przy użyciu bibliotek GTK+. Jak swój pierwszy projekt stworzyłem prosty komunikator sieciowy działający na zasadzie klient-serwer. Chciałbym zatem prosić o przetestowanie programu i przejrzenie kodu jeśli ktoś ma ochotę i zna się na gtk. Będę wdzięczny za wszelkie wskazówki i poprawki.

Instalacja jest bardzo prosta - rozpakowujemy archiwum i wykonujemy polecenie make. W razie problemów z kompilacją powinno pomóc doinstalowanie biblioteki libgtk2.0-dev. Potem aplikację uruchamiamy poleceniem ./netcom. Na jednym komputerze uruchamiamy program w trybie serwera, a na drugim (albo na tym samym) odpalamy go w trybie klienta, wpisujemy ip serwera i możemy rozmawiać

WERSJA TRZECIA
Download http://www.speedyshare.com/files/21982997/netcom.tar.gz

[el.pescado]

Program ciekawy, ale mam kilka uwag.

1. Nie używaj GtkFixed - to jest najgorszy sposób projektowania okien, który powinien być używany tylko w ostateczności. Zamiast tego, naucz się korzystać z pojemników (containers) - głównie GtkVBox, GtkHBox i GtkTable. Dzięki temu uzyskasz dynamiczny ukłas widgetów, który będzie się dobrze skalował. Używanie pojemników to jest podstawa GTK+.

2. Program chyba nie obsługuje nazw hostów - nie można się połączyć z "localhost", podczas gdy "127.0.0.1" działa. Spróbuj się pobawić z gethostbyname() albo modułu GResolver z biblioteki GIO.

3. Próbowałbym uniknąć używania wątków. GUI i wątki zwykle oznacza kłopoty. Trzeba pamiętać o tym że na niektórych platformach (Win32) w ogóle nie można się odwoływać do GUI z wątku innego niż główny. Przepisałbym aplikację tak, by korzystała z nieblokującego wejścia/wyjścia, np. za pomocą funkcji g_io_add_watch (). Przydaje się też zapoznać z systemowymi funkcjami select() i poll().

4. Żeby zwiększyć przenośność kodu, zamiast interfejsu gniazdek Berkeley (socket(), connect() itd), można skorzystać z bibliotek wyższego poziomu, jak np. GNet albo (lepiej) GIO.

5. Aktualnie kod sieciowy jest wymieszany z kodem odpowiadającym za GUI. Dobrze byłoby je wydzielić do osobnych modułów. Zbyt dużo jest też zmiennych globalnych. Dobrze byłoby utworzyć struktury zawierające wszystkie potrzebne dane i przekazywać wskaźniki do nich jako argumenty funkcji.

6. Przy funkcjach brakuje komentarzy;)

7. Na koniec można się nauczyć biblioteki GObject i pisać prawdziwie obiektowe programy w C.

Poza tym OK:)

[dawwin]

Postaram się wziąć pod uwagę Twoje wskazówki
Odnośnie wątków sugerowałem się tym artykułem. Nigdy nie pisałem aplikacji okienkowych pod Win32, więc nie wiedziałem, że się nie da

[el.pescado]

http://library.gnome.org/devel/gdk/unst ... escription

Unfortunately the above holds with the X11 backend only. With the Win32 backend, GDK calls should not be attempted from multiple threads at all.

[dawwin]

Zastosowałem się do Twoich wskazówek. Usunąłem wątki, dodałem obsługę nazw hostów, wydzieliłem kod sieciowy do osobnego pliku, wstawiłem kontenery do 'chat window'. Komentarze do kodu będą później. Wersja poprawiona jest dostępna w pierwszym poście

[el.pescado]

Wygląda lepiej:) choć komentarze rzeczywiście by się przydały. Naprawdę dobrze jest wyrobić sobie nawyk ich pisania. Czasami wręcz powinno się komentować kod jeszcze przed jego napisaniem.

Co do kodu, znalazłem kilka wywołań funkcji strcpy(), która jest potencjalnie niebezpieczna, gdyż nie sprawdza wielkości bufora, przez co łatwo, przez nieuwagę, dopuścić błąd przepełnienia bufora. Chociaż widzę że wszędzie w kodzie alokowany jest bufor odpowiedniej wielkości, dobrze jest dmuchać na zimne i zamiast strcpy() używać strncpy(). Poza tym, warto skorzystać z dobrodziejstw biblioteki GLib, które potrafią uprościć manipulowanie napisami:

Kod: Zaznacz cały

		clen = strlen(centrytxt);
		gl_clientip = (char *) malloc((clen + 1) * sizeof(char));
		strcpy(gl_clientip, centrytxt);

Można zastąpić:

Kod: Zaznacz cały

gl_clientip = g_strdup (centrytext);

Kod: Zaznacz cały

		entrytxt = gtk_entry_get_text(gf_chatentry);
		if (check_string(entrytxt) == 0) {
				return;
		}
		
		entlen = strlen(gl_nickname) + strlen(entrytxt) + ADDITIONAL_CHARS;
		if (entlen > BUFFER) {
				gtk_entry_set_text(gf_chatentry, "");
				showmessage("Message is too long", GTK_MESSAGE_ERROR);
				return;
		}
		
		strcpy(message, gl_nickname);
		strcat(message, "> ");
		strcat(message, entrytxt);
		strcat(message, "\n");
		
		add_line(message);

Można skrócić używając g_strdup_printf:

Kod: Zaznacz cały

entrytxt = gtk_entry_get_text(gf_chatentry);
char *message = g_strdup_printf ("<%s> %s\n", gl_nickname, entrytxt);
add_line (message);
g_free (message);

Na koniec zachęcam do zapoznania się z jakimś systemem kontroli wersji, jak svn czy git, oraz założeniem strony projektu na jakimś serwisie hostującym, w stylu SourceForge, Google Code czy github.

[dawwin]

Jak widzę GLib ma przede mną jeszcze wiele tajemnic . Pokombinuję jeszcze z g_strdup(). Jak poprawie to wrzucę tu nową wersję

Edycja:
Wersja trzecia jest dostępna w pierwszym poście