✔ Begutachtung von Source

So, nachdem ich jetzt das Einlesen von STDIN umgebaut habe, sodass in 1KB-Bloecken gelesen wird hat sich die Ausfuehrungszeit des Scripts schonmal um ein paar Sekunden gebessert. Bei meiner 6.5MB grossen Testmail liegt der Unterschied bei ca. 7 Sekunden.

Nun habe ich auch versucht an der Ausgabe was zu feilen, dazu hab ich mal mit time gemessen wie lange diese beiden Anweisungen brauchen um verschiedene eMails auszugeben.
Code: 
printf("%s",email);
oder so
Code: 
fwrite(email,sizeof(char),strlen(email),stdout);
ausgebe.
Eigentlich sollte ja, meinem Verstaendnis nach, die obere Version wegen eventueller Formatierung etwas langsamer sein, oder?
Bei kleinen Dateien scheint das wohl nicht der Fall zu sein, denn eine knapp 2 KB grosse eMail wird mit printf() ein klein wenig schneller ausgegeben als mit fwrite().
Die beiden anderen eMails, eine mit 1.9MB, eine mit 6.5MB, waren jeweils bei der Ausgabe mit fwrite() etwas schneller.
Alles in allem scheint es aber keinen sehr grossen Unterschied zu machen, jedoch denke ich, dass fwrite() evtl. besser skaliert wenn es sich um grosse Datenmengen bei der Ausgabe handelt.

Ich hatte auch probiert in den meisten Faellen strncat() durch strncpy() zu ersetzen. Was aber wieder dazu fuehrte, das die Mail ziemlich zerwurstet ausgegeben wurde.
Das war auch der Grund warum ich von strncpy() ueberhaupt zu strncat() gewechselt bin.
Hier mal ein kleines Beispiel.
Original:
From: test@test.de
X-KMail-Transport: LocalMail
To: test@test.de
Subject: test-clean
Date: Thu, 22 Dec 2005 15:57:30 +0800
User-Agent: KMail/1.7.2
X-KMail-CryptoFormat: 15
MIME-Version: 1.0
Content-Type: text/plain;
charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
X-Original-Status: R
X-Original-X-Status: N
X-KMail-EncryptionState: N
X-KMail-SignatureState: N
X-KMail-MDN-Sent:
X-Original-X-UID: 21
X-Length: 560
X-Original-Status: RO
X-Original-X-Status: RSC
X-UID: 25

test
Zum Vergrößern anklicken....
strncat():
From: test@test.de
X-KMail-Transport: LocalMail
To: test@test.de
Subject: test-clean
X-Virus-Status: No
X-Virus-Checker: Scanned by mailscan 0.9 (No virus found)
Date: Thu, 22 Dec 2005 15:57:30 +0800
User-Agent: KMail/1.7.2
X-KMail-CryptoFormat: 15
MIME-Version: 1.0
Content-Type: text/plain;
charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
X-Original-Status: R
X-Original-X-Status: N
X-KMail-EncryptionState: N
X-KMail-SignatureState: N
X-KMail-MDN-Sent:
X-Original-X-UID: 21
X-Length: 560
X-Original-Status: RO
X-Original-X-Status: RSC
X-UID: 25

test
Zum Vergrößern anklicken....
strncpy():
From: test@test.de
X-KMail-Transport: LocalMail
To: test@test.de
Subject: test-cle
j@X-Virus-Status: No
X-Virus-Checker: Scanned by mailscan 0.9 (No virus found)
Date: Thu, 22 Dec 2005 15:57:30 +0800
User-Agent: KMail/1.7.2
X-KMail-CryptoFormat: 15
MIME-Version: 1.0
Content-Type: text/plain;
charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
X-Original-Status: R
X-Original-X-Status: N
X-KMail-EncryptionState: N
X-KMail-SignatureState: N
X-KMail-MDN-Sent:
X-Original-X-UID: 21
X-Length: 560
X-Original-Status: RO
X-Original-X-Status: RSC
X-UID: 25

test

Ñ
Zum Vergrößern anklicken....
Es sollte recht offensichtlich sein, dass bei der letzten Version der Betreff abgeschnitten ist, die folgende Zeile mit ziemlichem Unsinn anfaengt und auch am Ende der eMail noch irgendein Unsinn angefuegt wurde.
 
Code: 
Es sollte recht offensichtlich sein, dass bei der letzten Version der Betreff abgeschnitten ist, die folgende Zeile mit ziemlichem Unsinn anfaengt und auch am Ende der eMail noch irgendein Unsinn angefuegt wurde.

Wie benutzt du denn strncpy ?

Wenn der src String von strncpy nicht mit einer \0 abgeschlossen ist muss man den
dest String selber mit \0 abschließen. Siehe dazu auch:
http://www.cplusplus.com/ref/cstring/strncpy.html


Gruß

RedWing
 
Hmm, da werd ich dann mal was rumprobieren muessen.
Im Grunde hab ich einfach nur strncat() durch strncpy() ersetzt. Werd mal schauen, dass ich am Ende noch terminiere und gucken was dann dabei rumkommt.

Nachtrag:
Scheint zu funktionieren.
Der Code dazu sieht jetzt so aus:
Code: 
/*email[0]=(char)NULL;
strncat(email,data,strlen(data)-strlen(part2+1));*/
strncpy(email,data,strlen(data)-strlen(part2+1));
email[strlen(data)-strlen(part2+1)]=(char)NULL;
Der auskommentierte Part ist der Code wie ich es zuvor gehandhabt habe.
 
Hi.

Da wirst du vermutlich irgendwas falsch gemacht haben. Wie wäre es denn erstmal mit einer kleinen Optimierung um die ganzen Kopien überhaupt zu eleminieren?

Code: 
char* subject_line;

subject_line = strstr (data, "Subject:");

subject_line = strchr(subject_line, '\n');

fwrite(data, sizeof(char), subject_line + 1 - data, stdout);

/* zusätzliche Header ausgeben ... */

fputs(subject_line + 1, stdout);

Die Überprüfung auf Fehler hab ich jetzt mal weggelassen.

Was das Schreiben angeht, wird in der Zeile
Code: 
fwrite(email,sizeof(char),strlen(email),stdout);
die Email 2x durchgelesen - einmal um die Länge der Email zu bestimmen und dann nochmal bei der Ausgabe. Du kannst ja mal fputs probieren ob das schneller ist. Allerdings nehmen sich die 2 Varianten mit printf / fputs sicherlich nicht viel da beim printf lediglich anfangs der Formatstring gelesen wird und das braucht nicht soviel Zeit.

Gruß
 
Die Eliminierung der Kopien werde ich dann morgen mal in Angriff nehmen, dann braucht das Programm wenigstens was weniger Speicher. :)
 
So, die Variable email konnte ich nun komplett rausschmeissen, jedoch ist laut top der Speicherverbrauch gleich geblieben. Bei einer 16MB Testmail werden mal eben 45MB (bei Ausgabe am Bildschirm, 30MB bei Umleitung in eine Datei per >) belegt.

Ich hab das zwar jetzt nicht ganz so gemacht wie von Dir vorgeschlagen, aber eigentlich sollte doch ein Unterschied feststellbar sein, immerhin faellt die groesste Variable (Groesse der Eingabe + 1KB) weg.
part1 habe ich weiter oben entfernt und belege nun direkt part2.
Anstelle von email nutze ich nun part1, jedoch nicht in der Groesse wie eMail sondern nur wie benoetigt. Weiterhin wird nicht alles an part1 angehaengt sondern gleich ausgegeben.
Allgemein sollte part1 jetzt auch wesentlich kleiner sein als zuvor, da dort nur noch die Header bis zum Subject enthalten sind und nicht mehr alles ab dem Subject.

Wie gehabt wird jede Variable freigegeben sobald sie nicht mehr benoetigt wird.

Alles in Allem wuerde ich behaupten, dass anhand des Codes eigentlich eine Kopie weniger im Speicher existiert und part1 durch die neue Rolle wesentlich kleiner geworden ist.

Ich hatte jetzt grad noch eine Idee wie ich part2 loswerden koennte. Ich werd mal rumprobieren und schauen was dabei rumkommt.

Dies ist der aktuelle Code, mit strncpy() und ohne email.
Code: 
Entfernt, weil schon wieder geaendert.

Nachtrag:
Ich habe part2 aus dem Code entfernen koennen und dadurch zumindest einen Teilerfolg verbuchen koennen.
Das Programm benoetigt fuer die selbe eMail (16MB) nun immer 30MB Speicher, ob nun auf den Bildschirm ausgegeben oder die Ausgabe in eine Datei umgeleitet wird.
Aber in letzterem Fall hat ja auch schon die alte Variante, die die noch eine komplette Kopie der auszugebenden eMail im Speicher erstellt hat, "nur" 30MB benoetigt.

Der Code dazu sieht nun so aus:
Code: 
/***************************************************************************
 *   Copyright (C) 2005 by Dennis Wronka                                   *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
 ***************************************************************************/


#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <stdio.h>
#include <stdlib.h>
#include <clamav.h>

const char mailscanversion[5]="0.94";
int ret;
unsigned int signo;
unsigned long memsize;
struct cl_node *node;
struct cl_limits limits;
const char *virname;
char buffer[1024];
char *data,*part1;
FILE *scantmpfile;

int main(int argc, char *argv[])
{
	ret=cl_loaddbdir(cl_retdbdir(),&node,&signo);
	if (ret)
		{
			return EXIT_FAILURE;
		}
	ret=cl_build(node);
	if (ret)
		{
			cl_free(node);
			return EXIT_FAILURE;
		}
	scantmpfile=tmpfile();
	if (scantmpfile==NULL)
		{
			cl_free(node);
			return EXIT_FAILURE;
		}
	while (fread(buffer,sizeof(char),sizeof(buffer),stdin))
		{
			fwrite(buffer,sizeof(char),strlen(buffer),scantmpfile);
			memset(&buffer,0,sizeof(buffer));
		}
	memsize=ftell(scantmpfile);
	fflush(scantmpfile);
	memset(&limits,0,sizeof(struct cl_limits));
	limits.maxfiles=1000000;
	limits.maxfilesize=20*1048576;
	limits.maxreclevel=100;
	limits.maxratio=500;
	limits.archivememlim=0;
	ret=cl_scandesc(fileno(scantmpfile),&virname,NULL,node,&limits,CL_SCAN_STDOPT | CL_SCAN_BLOCKMAX);
	data=malloc(memsize+1);
	if (data==NULL)
		{
			fclose(scantmpfile);
			cl_free(node);
			return EXIT_FAILURE;
		}
	data[0]=(char)NULL;
	rewind(scantmpfile);
	fread(data,sizeof(char),memsize,scantmpfile);
	data[memsize]=(char)NULL;
	fclose(scantmpfile);
	if ((strstr(data,"Subject:")==NULL) || (strstr(strstr(data,"Subject:"),"\n")==NULL))
		{
			free(data);
			cl_free(node);
			return EXIT_FAILURE;
		}
	part1=malloc(strlen(data)-strlen(strstr(strstr(data,"Subject:"),"\n")+1)+1);
	if (part1==NULL)
		{
			free(data);
			cl_free(node);
			return EXIT_FAILURE;
		}
	strncpy(part1,data,strlen(data)-strlen(strstr(strstr(data,"Subject:"),"\n")+1));
	part1[strlen(data)-strlen(strstr(strstr(data,"Subject:"),"\n")+1)]=(char)NULL;
	fputs(part1,stdout);
	free(part1);
	if (ret==CL_VIRUS)
		{
			printf("X-Virus-Status: Yes\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (Virus found: %s)\r\n",mailscanversion,virname);
		}
	else if (ret==CL_CLEAN)
		{
			printf("X-Virus-Status: No\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (No virus found)\r\n",mailscanversion);
		}
	else
		{
			printf("X-Virus-Status: Error\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (Scan error: %s)\r\n",mailscanversion,cl_strerror(ret));
		}
	fputs(strstr(strstr(data,"Subject:"),"\n")+1,stdout);
	free(data);
	cl_free(node);
	return EXIT_SUCCESS;
}
 
Dennis Wronka hat gesagt.:
Das Programm benoetigt fuer die selbe eMail (16MB) nun immer 30MB Speicher, ob nun auf den Bildschirm ausgegeben oder die Ausgabe in eine Datei umgeleitet wird.
Aber in letzterem Fall hat ja auch schon die alte Variante, die die noch eine komplette Kopie der auszugebenden eMail im Speicher erstellt hat, "nur" 30MB benoetigt.
Zum Vergrößern anklicken....
Ok, also ich hab das jetzt auch nochmal ausprobiert. Meine (nicht-infizierte) Datei war dabei 22MiB groß, ich hab es unter Cygwin laufen lassen und konnte folgendes beobachten:

Die frühere Variante (http://www.tutorials.de/forum/showpost.php?p=1209538&postcount=15) gönnte sich max. ganze 76 MiB Speicher.

Die neueste Version benötigt lediglich 32MiB, wobei ca. 10MiB für ClamAV draufgehen sobald es initialisiert ist. (also logisch: 10 MiB + 3 * 22 MiB = 76 MiB)

Das alte Programm brauchte relativ lange (1m 36s) um die Mail zu scannen, das neue nur 37 sek. Meine veränderte Version (so wie in der letzten Nachricht) braucht ca. 3 sek. weniger.

Das Lesen der Daten ist auch noch nicht wirklich gut weil keine Fehler abgefangen werden und nicht geprüft wird wieviel Zeichen gelesen wurden.

Code: 
size_t num;

while ((num = fread(buffer,sizeof(char),sizeof(buffer),stdin)) != 0)
{
	if (fwrite(buffer,sizeof(char),num,scantmpfile) != num) {
		puts("Fehler beim Schreiben in temp. Datei.\n", stderr);
		fclose(scantmpfile);
		return EXIT_FAILURE;
	}
}
if (ferror(stdin)) {
	fputs("Fehler beim Lesen der Standardeingabe.\n", stderr);
	fclose(scantmpfile);
	return EXIT_FAILURE;
}

Code: 
if (fwrite (data, sizeof(char), line - data + 1, stdout) != (line - data + 1)) {
	free(data);
	return EXIT_FAILURE;
}

Gruß
 
Womit ueberpruefst Du eigentlich den Speicherverbrauch?
Ich schaue dazu waehrend der Ausfuehrung in top wieviel das Programm sich zieht.

Und was genau meinst Du damit?
Meine veränderte Version (so wie in der letzten Nachricht) braucht ca. 3 sek. weniger.
Zum Vergrößern anklicken....

Das hier?
Code: 
char* subject_line;

subject_line = strstr (data, "Subject:");

subject_line = strchr(subject_line, '\n');

fwrite(data, sizeof(char), subject_line + 1 - data, stdout);

/* zusätzliche Header ausgeben ... */

fputs(subject_line + 1, stdout);
Zum Vergrößern anklicken....
 
Dennis Wronka hat gesagt.:
Womit ueberpruefst Du eigentlich den Speicherverbrauch?
Zum Vergrößern anklicken....
Dazu hab ich den ProcessExplorer genommen (wie gesagt mach ich das gerade unter Cygwin) und ich hab in das Programm einen sleep Befehl eingebaut damit ich auch etwas Zeit habe zu sehen wieviel Speicher genutzt wird. Den sleep Aufruf hab ich natürlich genau da gesetzt wo der meiste Speicher alloziert ist - das ist zwar nur ein kurzer Augenblick, aber irgendwo muß er ja herkommen und wenn das System ausgelastet ist, kann das schon mal ein wenig dauern bis der kernel Platz im virt. Speicher freigeschaufelt hat.

Dennis Wronka hat gesagt.:
Und was genau meinst Du damit?
deepthroat hat gesagt.:
Meine veränderte Version (so wie in der letzten Nachricht) braucht ca. 3 sek. weniger.
Zum Vergrößern anklicken....
Zum Vergrößern anklicken....
Code: 
/***************************************************************************
 *   Copyright (C) 2005 by Dennis Wronka                                   *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
 ***************************************************************************/


#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <stdio.h>
#include <stdlib.h>
#include <clamav.h>

const char mailscanversion[5]="0.94";
int ret;
unsigned int signo;
unsigned long memsize;
struct cl_node *node;
struct cl_limits limits;
const char *virname;
char buffer[1024];
char *data;
FILE *scantmpfile;

int main(int argc, char *argv[])
{
	size_t num;
	
	ret=cl_loaddbdir(cl_retdbdir(),&node,&signo);
	if (ret)
		{
			return EXIT_FAILURE;
		}
	ret=cl_build(node);
	if (ret)
		{
			cl_free(node);
			return EXIT_FAILURE;
		}
	scantmpfile=tmpfile();
	if (scantmpfile==NULL)
		{
			cl_free(node);
			return EXIT_FAILURE;
		}
	
	while ((num = fread(buffer,sizeof(char),sizeof(buffer),stdin)) != 0)
		{
			if (fwrite(buffer,sizeof(char),num,scantmpfile) != num) {
				fputs("Fehler beim Schreiben in temp. Datei.\n", stderr);
				fclose(scantmpfile);
				return EXIT_FAILURE;
			}
		}
	if (ferror(stdin)) {
		fputs("Fehler beim Lesen der Standardeingabe.\n", stderr);
		fclose(scantmpfile);
		return EXIT_FAILURE;
	}
	
	memsize=ftell(scantmpfile);
	fflush(scantmpfile);
	memset(&limits,0,sizeof(struct cl_limits));
	limits.maxfiles=1000000;
	limits.maxfilesize=20*1048576;
	limits.maxreclevel=100;
	limits.maxratio=500;
	limits.archivememlim=0;
	ret=cl_scandesc(fileno(scantmpfile),&virname,NULL,node,&limits,CL_SCAN_STDOPT | CL_SCAN_BLOCKMAX);
	cl_free(node);

	data=malloc(memsize+1);
	if (data==NULL)
		{
			fclose(scantmpfile);
			return EXIT_FAILURE;
		}
	rewind(scantmpfile);
	num = fread(data,sizeof(char),memsize,scantmpfile);
	if (ferror(scantmpfile)) {
		fclose(scantmpfile);
		return EXIT_FAILURE;
	}
	data[num+1]= '\0';
	fclose(scantmpfile);
	
	char* line = strstr(data,"Subject:");
	
	if (line == NULL || (line = strchr(line,'\n'))==NULL)
		{
			free(data);
			return EXIT_FAILURE;
		}
	if (fwrite (data, sizeof(char), line - data + 1, stdout) != (line - data + 1)) {
		free(data);
		return EXIT_FAILURE;
	}
	
	if (ret==CL_VIRUS)
		{
			printf("X-Virus-Status: Yes\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (Virus found: %s)\r\n",mailscanversion,virname);
		}
	else if (ret==CL_CLEAN)
		{
			printf("X-Virus-Status: No\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (No virus found)\r\n",mailscanversion);
		}
	else
		{
			printf("X-Virus-Status: Error\r\n");
			printf("X-Virus-Checker: Scanned by mailscan %s (Scan error: %s)\r\n",mailscanversion,cl_strerror(ret));
		}
	fputs(line+1,stdout);
	free(data);
	
	return EXIT_SUCCESS;
}

Gruß
 
Den Code werd ich mir morgen mal in aller Ruhe ansehen. Und auch mal nach ein paar weiteren nuetzlichen Debug-Tools schauen. ValGrind ist ja schon eine klasse Sache, aber ein paar andere Infos waeren auch nicht schlecht, und irgendwie ist dbg, bzw. kdbg, nicht das was ich wirklich brauche.
 
Um antworten zu können musst du eingeloggt sein.

Neue Beiträge

Zurück