nicht angemeldet
Hallo,
na, *das* nenne ich doch mal eine 'konstruktive Kritik' -- danke!
Jaja, da bemüht man sich und bekommt es dann noch um die Ohren gehauen.
Das Schlimmste ist ja: sogar noch zu Recht! ;-)
Zeile 0: bitte noch ein #include <unistd.h> einbauen (für getpid(),
unlink() und close())
Hm, bisher dachte ich, das waere in der stdlib.h definiert -- danke fuer den
Hinweis.
Weder getpid(), close() noch unlink() sind ANSI C 99.
unistd.h übrigens auch nicht ;-)
Macht auch normalerweise der Compiler selber, gibt halt nur bei 'gcc -pedantic -ansi -Wall' eine Warnung aus.
Kann auch sein, daß meine Libc zu alt ist (GLibc 2.0.irgendwas)
BTW: ist das evt keine gute Idee auf allzuviele nicht ANSI C Funktionen zurückzugreifen?
Achso: man müßte mal schauen, ob da Haken drin sind, die bei 64 Bit Architekturen hängen könnten. Sollte zwar nichts drin sein, aber ...
(Zeile 44 ff: der alte Macteiler ist IMHO ':', wenn's gebraucht werden
sollte)
Ich habe keinen Mac zum testen. Da muesste sich ein Mac-Freak hinsetzen und
das patchen.
Würde auch eh nur für die alten Mac OS < 10 gelten.
Und da einen Server drauf? Na, ich weiß ja nicht ;-)
Zeile 234: malloc() ohne Überprüfung? Ganz böse Falle, aber wirklich _ganz_
böse!
Find ich gar nicht. Wann sollte *heutzutage* noch ein malloc() fehlschlagen,
gerade wenn es nur um ein paar Bytes geht?
Da Du nicht weißt, wie malloc() auf der jeweiligen Architektur funktioniert, gibt es einige Gründe.
Kannst Dein malloc() an den meisten Stellen eh ganz weglassen.
(Hatte ich nicht von Dir neulich mal einen Preis bekommen für die "most useless pipe", oder war das der Michael? ;-)
und weder strcpy() noch strcat() sind eigentlich zu empfehlen.
Warum?
strcpy() weil in Deinem Fall tempfile = cfile (+ der danach notwendigen Änderungen) besser gewesen wäre, hätte auch die Speicherallokation gespart. (Nur die Adresse wird kopiert)
strcpy() kann in die Hose gehen, wenn Argument 1 kein regulärer C String ist (kein \0 am Ende).
Wenn Du nicht auf strcpy() verszichten willst, dann nimm wenigstens strncpy()
strcat() läßt sich dann auch auf fast gleiche Weise ersetzen und hat fast die gleichen Probleme mit fast der gleichen Lösung.
Vielleicht ist es aber auch nur Geschmackssache ;-)
Zeile 253: Überprüfung von fclose() fehlt.
Danke, guter Punkt! :-)
Jaja, ich und mein Lint ;-)
(Bin ich denn eigentlich nur blind? Ich bin da auch noch fast alles im Code nachgegangen. Nunja, "fast" alles trifft es da gut ;-)
Zeile 271: Memory Leak! gz ist noch nicht freigegeben vor dem return!
Bitte wie? 'gz' sollte doch fclose() freigeben, ich habe den Speicher
schliesslich nicht angefordert.
Ja und?
Und ich habe noch keinen Sourcecode gesehen,
wo ein FILE * freigegeben wird.
Stimmt, da habe ich mich wohl zu sehr auf mein Lint verlassen.
Scheint ein Bug (im Lint) zu sein, muß ich wohl nochmal bei.
'tschuldigung.
(2. Mal schon! Peinlich, peinlich!)
Zeile 343: sowas geht nicht gut. Hau lieber noch ein paar Klammern drumherum.
if ((gzip = strstr(gzip, "gzip"))) {
Noajoa, es waere gut gegangen. Aber -pedantic will halt noch eine Klammer
haben...
;-)
Aber ist immer besser. Wer weiß, welcher Compiler da dran geht. Kann ja nicht immer GCC sein ;-)
Zeile 348: *gzip != 0 passt nicht. Die Stelle auf die der Pointer zeigt
ist Type char, 0 ist Type int.
Zum Glueck weiss der Compiler das auch und macht daraus einen char :-)
Was suchst Du da eigentlich? \0, also Ende vom String?
NULL, also Ende vom Speicher?
'0', die Zahl Null als Buchstabe?
Dann schreib das doch einfach ;-)
Zeile 443, 450, 458, 525, 532, 540: tptr kann NULL sein, verträgt sich mit
strftime() nicht.
Hm. Warum sollte tptr null sein koennen? Bzw. in welchem Fall? Aber ich habe
es beruecksichtigt jetzt, danke.
time() kann versagen, localtime() kann versagen.
Irgendwie sieht das sowieso komisch aus. Ich mache bei time() immer:
time_t t;
time(&t);
time(NULL) habe ich noch nie gesehen ;-)
BTW: NULL kann auch als (void)(0) definiert sein. (Ich z.B. mache das immer) Das passt dann nicht in time() rein. Gibt zumindest 'ne Warnung.
Zeile 599: close() will als erstes Argument ein int, bekommt aber eine FILE*
Beschwert sich übrigens auch der GCC drüber!
Aber hast Du wahrscheinlich nicht verstanden, oder?
Doch, natuerlich ;-) Ich habe nur nicht mit -Wall compiliert,
Tststs ;-)))
BTW: welche Optimierung verträgt er eigentlich?
Schonmal ausprobiert?
zum Schluss. Das
mit der 'filename.conf' war mehr oder weniger ein Release-Quickhack, damit
ich bei den Binaries nicht den Cache-Pfad fest einkompilieren muss.
Könnte ja jetzt sagen: "So sieht's auch aus!", aber wer bin ich denn ;-)
Dadurch hast Du aber auch noch eine globale Variable, die Du ganz schlecht wegbekommst. (Die andere (LgFile) ist einfach)
Zeile 632: may_gzip() ist weder bool noch int.
Doch, may_gzip() ist
int may_gzip(void)
Ja, selbst zu blöd zum abschreiben bin ich:
gzip_cncc.c:632:6: Test expression for if not boolean, type int: may_gzip()
steht da ja auch! Und ich wollte gerade wieder meinem Lint die Schuld in die Schuhe schieben ;-)
(Das void steht bei mir aber nicht! ;-)
Nach Reparatur von 343 und 599 hat
gcc -Wall -pedantic -ansi gzip\_cncc.c -lzgar keine Beschwerden mehr.
Dito.
Ich kannte -pedantic und -ansi vorher gar nicht. -Wall sind anscheinend doch
nicht alle Warnungen?
Nein, noch lange nicht ;-)
Wenn Du das Revidierte oben hast, sag doch mal eben Bescheid.
Dann geh ich da mal in Ruhe durch.
Und vor allem zu Fuß!
Scheiß Lint!
*grummel* ;-)
BTW: Bennen Doch bitte Dein Makro EXPIRE um, alle Makros mit dem Anfagsbuchstaben "E" sind reserviert. Mach einfach CNC_EXPIRE o.ä.
so short
Christoph Zurnieden
PS: sehe ich gerade. Schau doch mal ganz oben in die Zeile, wo Dein Name steht.
Ja, genau ;-)
CZ
PPS:
Warum meinst Du eigentlich, das eine PID bis zu 1024 Stellen haben kann?
Nur so als Frage ;-)
CZ