Projektvorstellung: .. (Namensvorschläge werden gerne angenommen)
-
Guten Tag,
Ich möchte hier ein kleines Projekt vorstellen mit dem man Dateien relativ simpel über TCP Sockets übertragen kann. Ich bin mir durchaus bewusst dass es derartige Programme schon gibt, aber mit irgendetwas muss man ja anfangenWo wir auch schon bei einem wichtigen Punkt wären:
Warum veröffentliche ich das?Ganz einfach, ich bin noch Schüler und schreibe seit ein paar Monaten ab und an mal ganz gerne kleinere Programme mit C (C++). Allerdings habe/kenne ich niemanden der in der Lage wäre meinen Code hinsichtlich folgender Aspekte zu beurteilen:
- Lesbarkeit
- Sicherheit
- Logik (Unsinnige Stellen im Code?)
- Was sagt der Standard (meist nicht viel, manchmal zu viel^^)
- Sauberkeit
usw usw..
Kurz gesagt, ich habe einfach keine Rückmeldung (außer die meines Compilers) ob der geschriebene Code so vertretbar ist, was man besser machen könnte, was man vielleicht auch unbedingt vermeiden sollte.
Da hier im Forum oft ein ziemlich.. sarkastischer Ton herrscht (was beim lesen köstlich ist, aber die armen Threadersteller trauen sich ja kaum noch etwas zu schreiben) bitte ich hier doch direkt noch mal darum Kritik positiv zu formulieren. zB:
statt: "Der Code ist völlig unsauber und nicht zu gebrauchen!"
lieber: "Der Code ist jetzt vielleicht nicht ganz sauber, allerdings könnte man ihn bestimmt in 5-7 Stunden debuggen! ;)"In diesem Sinne: Vielen Dank für das Lesen meines Codes und besten Dank für eure Kritik schonmal im vor(-r)aus
http://www10.zippyshare.com/v/31475174/file.html
Link enthält Zip-Archiv mit release.exe und den source files.
Diesen Hoster finde ich praktisch, keine Anmeldung & keine Wartezeit.
Falls ein Upload auf sourceforge gewünscht ist melde ich mich dort an,
habe da aber gerade keine Lust zuPS: Wer das Programm im Internet testet sollte daran denken im Router die entsprechenden Ports freizuschalten usw.
Edit1: In Beta v1 haben sich am Schluss doch tatsächlich zwei kleine bugs eingeschlichen naja, sind behoben und Link ist aktualisiert.
Edit2: In Zeile 235 von "Client_Send.cpp" wird die Konstante ID_DI_SERVER_HOST_FILESIZE anstatt von ID_DI_CLIENT_SEND_FILESIZE benutzt. Lade bei Gelegenheit neue Version hoch
Edit3: Beta v1.2 ist fertig, es wird jetzt gestattet Dateien mit mehr als 2GB größe zu übertragen, allerdings stimmt die "Bytes" Anzeige ab einer Dateigröße von 4GB nicht mehr. Link aktualisiert.
Edit4: Gibts sonst keine Kritik oder niemanden der Zeit genug hat / gewillt ist sich die mühe zu machen den Code zu lesen?
-
Hey cooky451,
ich habe mir gerade dein Projekt gedownloaded und sehe es mir gerade ein bissel an. Was mir sofort bei dir auffällt ist, dass du stark an C hängst
Das soll nicht negativ klingen, aber in so ziemlich allen Büchern, die ich gelesen habe, steht das auf goto Befehle komplett verzichtet werden soll! Dafür gibt es Schleifen und den ganzen andren Krämpel^^
Ich nicht, ob du demnächst vor hast auf C++ umzusteigen oder nicht, wenn doch, dann solltest du die ganzen #define Variablen in const Variablen umändern.Ansonsten kann ich sagen, dass du übersichtlich gearbeitet hast. Die Unterteilung der einzelnen Dateien und auch durch die verschiedenen Abschnitte innerhalb der Datei sind gut nachzuvollziehen. Allerdings hättest du noch ein paar mehr Kommentare einfügen können, dann sind deine Gedanken leichter für andre nachzuvollziehen.
Das ist es erstmal soweit zum Aussehen.
Den eigentlichen Inhalt muss ich mir erstmal anschauen. Das kann etwas dauern, da dein Projekt ja nicht ganz klein istDas wäre erstmal so mein erster Eindruck der ganz positiv ist. Hoffe das einige andere dir auch noch schreiben, vor allem bezüglich des Inhalts.
-
Hi BootLag,
besten Dank für deine Antwort
Ja, ich hänge in diesem Fall stark an C da man mir sagte ich solle versuchen C und C++ immer mehr oder minder zu trennen^^ Deswegen habe ich versucht möglichst auch wirklich nur C zu benutzen. (Konnte bei diesem Projekt in C++ keinen Vorteil finden)Das goto Zeugs hab ich gemacht um 1. immer die Freigabe aller Ressourcen vor Beendigung des Threads zu garantieren und 2. weil ich auch keine komfortable Möglichkeit gesehen habe die Rückgabewerte des Threads irgendwo außerhalb auszuwerten
Das man goto nie niemals benutzen soll ist mir bekannt^^, allerdings meist mir der Begründung dass das Programm bei vielen goto Befehlen unübersichtlicher wird als mit Schleifen, in diesem Fall aber glaube ich gerade der goto Befehl einiges an Übersicht mit sich bringt. Falls man so etwas trotzdem unbedingt vermeiden sollte bin ich für Änderungsvorschläge offen!Zur Betrachtung des "echten" Code Inhalts vielleicht ein paar Anmerkungen die das erleichtern:
Die Codes sind gewissermaßen gespiegelt, es gibt:
- Eine "Hauptroutine" für die "Clienten" und eine für die "Server".
- Diese unterscheiden sich nur in:
- Der Art, wie sie sich die erwartete Dateigröße beschaffen (Der Sender, egal ob Server oder Client kann diese ja auslesen)
- Der Transferschleife, wobei diese auch wieder gespiegelt ist es gibt also im Prinzip nur EINE Sendeschleife und EINE Empfangsschleife.Vielleicht hilft das etwas, dann wird der Code gleich übersichtlicher
Edit: Zu den "anderen" die scheinen heute ja frei zu haben, recht wenig los hier heute
-
Ich weiß man soll nicht unfreundlich sein^^,
aber deine Kommentare sind furchtbar./** --------------------------------------------------------------------------------------------------- *** Dialog *** --------------------------------------------------------------------------------------------------- **/
Ich finde sowas nervt extrem
/* Dialog */
tuts auch (vor allem da jeder gute Compiler Syntaxhighlighting macht und Kommentare sowieso ins Auge springen)
Außerdem müssen Zeilen nicht so lang sein, gerade wenn man sie auf Linux innem Terminal guckt ist das sehr störend
80 zeichen sind ne gute Richtlinie (man muss nich dran kleben aber man sollte immer nen guten Grund haben wieso man längere Zeilen macht)
EDIT: inhaltlich kann ich zu deinem Code leider wenig sagen da ich die Win-Programmierung nicht kenne
-
Nunja, man sollte bedenken dass das Programm ganz klar für Windows geschrieben ist. Ich nutze solche etwas übertriebenden Kommentare eigentlich nicht, allerdings hilft es mir bei WINAPI Projekten irgendwie die Übersicht zu bewahren (Ich finde WINAPI nicht gerade sehr übersichtlich)
Bei portablen Projekten nutze ich eher Kommentare wie
/* * Kommentar * */
Wäre so was besser?
Auf das mit den ca 80 Zeichen / Zeile wäre ich gar nicht gekommen, danke für den Tipp. Allerdings bin ich auch hier versucht die "Schuld" der WINAPI zu geben, wo sonst gibts so schrecklich lange Befehle?
-
Es ist ja nicht so das ich für kommentarlosen Code wäre^^(grauenhafte Vorstellung)
Es ist denk ich wichtig dass man verschiedene Sorten Kommentare hat
sowas:/******************************************************************** * extrem wichtiger kommentar * ********************************************************************/
(wenns über 80 sind is schlecht, hab se nit gezählt)
sowas/* * funktionsprototyperklärung */
aber auch sowas
/* dirty hack but works */ char *buf;/* buffer for file reading */
wiegesagt 80 ist ne richtlinie (und die langen Windowsbefehle sind ein guter Grund für Linux )
-
Gibts sonst keine Kritik?
Mich würde besonders interessieren ob es möglich ist eventuelle Schwachstellen in dieser Software zu nutzen um Pufferüberläufe oder sonstiges zu generieren und sich somit Zugriff zum System eines Nutzers zu verschaffen.
Auch wenn solch ein Angriff auf eine Software die keine Sau kennt natürlich unrealistisch ist so würde es mich doch interessieren ob dies hier theoretisch möglich ist und wie man es vermeiden könnte.
Falls das Programm sicher ist - noch besser, aber dass könnt ihr mir ja dann sagen
-
Auch bei diesem Projekt hättest du die Vorteile von C++ nutzen können. Das Initialisieren und Freigeben von Ressourcen (Winsock, Sockets, Dateien) hätten Konstruktoren und Destruktoren komfortabel für dich übernehmen können.
Und das goto hättest du dir dann auch sparen können.
Auch beim Parsing der IP-Adresse wäre ein std::string sicher einfacher handbar gewesen.
Aber C++ scheint ja nicht so dein Ding zu sein, der Code sieht eher nach C aus (abgesehen vom bool).
Dann noch die Frage warum du lRetVal denn 0x100 zuweist? Warum schreibst du es nicht in Dezimalzahlen oder definierst dir wenigstens eine Konstante, damit man weiß was dieses 0x100 zu bedeuten hat. Solche sogenannten "Magic Numbers" tragen nämlich nicht zur Lesbarkeit bei.
Übrigens solltest du dir die Klammern nach return abgewöhnen. Ich empfinde es als beim Lesen störend und sehe auch keinen Vorteil darin.
Ansonsten enthält der Quellcode sehr viel WinAPI und ich kann dir daher auch nicht mehr zu sagen.