Was würdet ihr bei diesem C99 Code anders machen, kann man noch etwas optimieren

[C Anfänger]

und ist irgendetwas daran schlecht, ineffizient etc.?
Ist die Dokumentation bzw. Beschreibung dazu ausreichend bzw. ok?

Programmbeschreibung:
Das Programm liest max. 10000 Primzahlen aus einer Textdatei und speichert sie zur weiteren Verwendung in ein int Array.

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

#define DATEINAME "primzahlen.txt"
#define ANZAHL_PRIMZAHLEN 10000

int main()
{
    FILE *fp;
    int primzahlen[ANZAHL_PRIMZAHLEN];
    unsigned char ch;
    unsigned char stringbuffer[6]; // Zum speichern mehrerer Ziffern, die eine einzige Zahl darstellen sollen
    int counter = 0;
    int primecounter = 0;

    /* Fuelle Array fuer Primzahlen mit Nullen */
    for (int i= 0 ; i < ANZAHL_PRIMZAHLEN; i++)
    {
        primzahlen[i] = 0;
        // printf("%i, %i\n", i, primzahlen[i]);
    }

    /* Lese Primzahlen aus Datei und speichere sie in Primzahlen Array */
    fp = fopen(DATEINAME, "r");
    stringbuffer[0] = 0; /* Das erste Zeichen in Stringbuffer muß auf 0 gesetzt werden,
                            damit das erste Zeichen in Stringbuffer nicht ausversehen mit einem anderen Zeichen
                            initalisiert wird, da dies ansonsten später zu einem Programmfehler fuehren koennte. */
    if(fp != NULL)
    {
        while(!feof(fp))
        {
          ch = fgetc(fp);
          if (isdigit(ch))
          {
            stringbuffer[counter] = ch;
            counter++;

          }
          else
          {
            counter = 0;
            if (stringbuffer[0] != 0) // Ueberpruefe ob Strinbuffer gefuellt ist
            {
              // printf("%s\n", stringbuffer);
              // system("PAUSE");

              primzahlen[primecounter] = atoi(stringbuffer);
              primecounter++;
              stringbuffer[0] = 0;  /* Markiere Stringbuffer als leer,
                                       damit eine Zahl nicht ein zweites mal im
                                       primzahlen Array gespeichert wird */
            }
          }

        }

        fclose(fp);
    }
    else{
        printf("Die Datei %s existiert nicht!\n", DATEINAME);
    }

    /* Listet alle eingelesenen Primzahlen auf */
    primecounter = 0;
    do
    {
        printf("%i, %i\n", primecounter, primzahlen[primecounter]);
        primecounter++;
    } while (primzahlen[primecounter] != 0);

    return 0;
}

Falls ihr das Programm testen wollt, so findet ihr hier die ersten < 10000 Primzahlen bzw. alle Primzahlen von 2 - 100000:
http://de.wikibooks.org/wiki/Primzahlen:_Tabelle_der_Primzahlen_(2_-_100.000)

Es genügt die Zahlen einfach via Copy&Paste in eine Textdatei zu kopieren und unter dem Namen primzahlen.txt abzuspeichern.

Alternativ könnt ihr eine fertige primzahlen.txt Datei auch einfach hier Downloaden, wobei IMO Copy&Pasten und selber in einer Datei speichern schneller geht.
http://netload.in/datei21FjupljKm.htm

[C Anfänger]

PS:

Natürlich hätte ich auch ein Progamm schreiben können, daß die ersten 10000 Primzahlen auch einfach durch Berechnung findet, aber ich will den Code weiternutzen und brauche daher absolut zuverlässige Zahlen die schon richtig sind.

[C Anfänger]

EDIT:

Ich habe jetzt noch zwei Zeilen geändert.

Die Größe des strinbuffers definiere ich jetzt über eine DEFINE Anweisung,
daß macht es dann später IMO einfacher das Programm zu ändern,
denn es erleichtert den Überblick, IMO.

#define STRINGBUFFERGROESSE 6
...
unsigned char stringbuffer[STRINGBUFFERGROESSE]; // Zum speichern mehrerer  Ziffern, die eine einzige Zahl darstellen sollen

[....]

/* Fuelle Array fuer Primzahlen mit Nullen */
    for (int i= 0 ; i < ANZAHL_PRIMZAHLEN; i++)
    {
        primzahlen[i] = 0;
        // printf("%i, %i\n", i, primzahlen[i]);
    }

Das lässt sich mit einer Zeile erledigen. Dafür gibts memset.

Müsste in etwa so gehen:

memset(&primzahlen, 0, ANZAHL_PRIMZAHLEN);

[volkard]

Ich finde das Dateiformat schlecht. Ich würde nur die Differenzen als Bytes speichern.

[C Anfänger]

.... schrieb:

Müsste in etwa so gehen:

memset(&primzahlen, 0, ANZAHL_PRIMZAHLEN);

Hm, ich habe daß jetzt mal hinter meine for Schleife (ca. @ Zeile 25) eingefügt und als Zeichen anstatt einer 0 irgendeine Zahl z.B. 1 genommen.
Aber leider funktioniert es nicht.
Denn wenn es funktionieren würde, dann würde er unten bei der Ausgabe des ganzen Arrays bis 10000 hochzählen und am Ende nur noch z.b. Einsen ausgeben.

[C Anfänger]

volkard schrieb:

Ich finde das Dateiformat schlecht. Ich würde nur die Differenzen als Bytes speichern.

Ok, aber was machst du bei 2 Primzahlen die eine Differenz ergeben, die den Wert 255 (der max. Wert für das Byte) übersteigt?

Ansonsten ist die Idee natürlich insofern gut, wenn man nur Primzahlen mit kleinen Differenzen hat und Platz auf der Festplatte sparen will und die Daten auch schnell einlesen können möchte.

[....]

C Anfänger schrieb:

.... schrieb:

Müsste in etwa so gehen:

memset(&primzahlen, 0, ANZAHL_PRIMZAHLEN);

Hm, ich habe daß jetzt mal hinter meine for Schleife (ca. @ Zeile 25) eingefügt und als Zeichen anstatt einer 0 irgendeine Zahl z.B. 1 genommen.
Aber leider funktioniert es nicht.
Denn wenn es funktionieren würde, dann würde er unten bei der Ausgabe des ganzen Arrays bis 10000 hochzählen und am Ende nur noch z.b. Einsen ausgeben.

Sorry mein Fehler. Probiers mal so:

memset(&primzahlen, 0, sizeof(*primzahlen) * ANZAHL_PRIMZAHLEN);

[volkard]

C Anfänger schrieb:

volkard schrieb:

Ich finde das Dateiformat schlecht. Ich würde nur die Differenzen als Bytes speichern.

Ok, aber was machst du bei 2 Primzahlen die eine Differenz ergeben, die den Wert 255 (der max. Wert für das Byte) übersteigt?

Mußt Du beim Abspeichern testen und einen Fehler ausgeben, wenns doch nicht klappt.

Oder Du schaust vorher, ob es reicht.
http://mathworld.wolfram.com/PrimeGaps.html
Aha, bis 387 Millionen kommst Du schon.
Da außer der ersten Differenz alle gerade sind, kannste auch das Byte einlesen und dann verdoppeln, so kommst Du bis 303 Milliarden. Aber das will wohl keiner.

[....]

volkard schrieb:

C Anfänger schrieb:

volkard schrieb:

Ich finde das Dateiformat schlecht. Ich würde nur die Differenzen als Bytes speichern.

Ok, aber was machst du bei 2 Primzahlen die eine Differenz ergeben, die den Wert 255 (der max. Wert für das Byte) übersteigt?

Mußt Du beim Abspeichern testen und einen Fehler ausgeben, wenns doch nicht klappt.

Oder Du schaust vorher, ob es reicht.
http://mathworld.wolfram.com/PrimeGaps.html
Aha, bis 387 Millionen kommst Du schon.
Da außer der ersten Differenz alle gerade sind, kannste auch das Byte einlesen und dann verdoppeln, so kommst Du bis 303 Milliarden. Aber das will wohl keiner.

Volkard, hilf mir mal. Mein memset-Aufruf klappt nicht (falsche Werte). Ich programmiere eher in C++ und habe mit dieser Funktion eher wenig am Hut. Wie lautet der Aufruf korrekt? Oder wo liegt das Problem?

[volkard]

.... schrieb:

Wie lautet der Aufruf korrekt? Oder wo liegt das Problem?

Ich hätte wenn ich genug betrunken wäre genommen memset(primzahlen,0,sizeof(primzahlen)), was aber nichts anderes macht als memset(primzahlen, 0, sizeof(*primzahlen) * ANZAHL_PRIMZAHLEN); und vermutlich sogar memset(&primzahlen, 0, sizeof(*primzahlen) * ANZAHL_PRIMZAHLEN);

Aber nüchtern sehe ich, daß das vorherige Löschen des gesamten Arrays überhaupt nicht nötig ist.

[....]

Es scheint wohl daran zu liegen, dass memset nur für char, aber nicht für int funktioniert. Sorry.

[C Anfänger]

volkard schrieb:

Aber nüchtern sehe ich, daß das vorherige Löschen des gesamten Arrays überhaupt nicht nötig ist.

Doch, bei der Ausgabe des Arrays ist das nächste leere Feldelement nämlich die Abbruchbedingung.

Eine Alternative dazu, wäre die Anzahl der Primzahlen irgendwo zu speichern oder das Array dynamisch genau so groß zu machen, wie man für die Primzahlen braucht.
Aber das wäre dann wieder Aufwand der zumindest für meine Zwecke unnötig wäre, daher habe ich ein statisches Array.

[volkard]

C Anfänger schrieb:

Doch, bei der Ausgabe des Arrays ist das nächste leere Feldelement nämlich die Abbruchbedingung.

Dann würde es schon reicghen, beim Einlesen hinter die letze gelesene Primzahl eine 0 zu schreiben.

primzahlen[primecounter]=0;//neu
fclose(fp);

[C Anfänger]

volkard schrieb:

C Anfänger schrieb:

Doch, bei der Ausgabe des Arrays ist das nächste leere Feldelement nämlich die Abbruchbedingung.

Dann würde es schon reicghen, beim Einlesen hinter die letze gelesene Primzahl eine 0 zu schreiben.

primzahlen[primecounter]=0;//neu
fclose(fp);

Gute Idee, danke für den Tipp!

[C Anfänger]

Mit --pedantic als Compileroption meldet mir der GCC Compiler eine warning message bezügl. Zeile 60:

primzahlen[primecounter] = atoi(stringbuffer);

primzahltest.c:60: warning: pointer targets in passing arg 1 of `atoi' differ in signedness

Wenn ich nun obige Zeile in folgende ändere:

primzahlen[primecounter] = atoi((signed)stringbuffer);

Dann bekomme ich immer noch eine Warnmeldung:

primzahltest.c:60: warning: passing arg 1 of `atoi' makes pointer from integer without a cast

Wie kriege ich diese weg?

[volkard]

oben
char stringbuffer[6];
oder unten
primzahlen[primecounter] = atoi((char*)stringbuffer);

[C Anfänger]

Besten Dank, ich habe jetzt das (char*) unten hingesetzt und nun funktioniert es.

[Wutz]

C Anfänger schrieb:

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

#define DATEINAME "primzahlen.txt"
#define ANZAHL_PRIMZAHLEN 10000

main()
{
    FILE *fp;
    int primzahlen[ANZAHL_PRIMZAHLEN], zaehler=0;
    char zeile[20];

    /* Lese Primzahlen aus Datei und speichere sie in Primzahlen Array */
    fp = fopen(DATEINAME, "rt");
    if(fp != NULL)
    {
      while( fgets(zeile,sizeof zeile,fp) )
        if( 1==sscanf(zeile,"%d",&primzahlen[zaehler] )
          ++zaehler;
      fclose(fp);
    }
    else
        perror(DATEINAME);  

    /* Listet alle eingelesenen Primzahlen auf */
    primecounter = 0;
    while( primecounter<zaehler )
    {
        printf("%d, %d\n", primecounter, primzahlen[primecounter]);
        primecounter++;
    }
    return 0;
}

Sollte so laufen und ist auch kürzer.

[C Anfänger]

Die Primzahlen liegen in der Datei aber nicht als Primzahl pro Zeile vor, sondern
es sind mehrere Primzahlen innerhalb einer Zeile, getrennt von Leerzeichen und Komma.

[volkard]

C Anfänger schrieb:

PS:

Natürlich hätte ich auch ein Progamm schreiben können, daß die ersten 10000 Primzahlen auch einfach durch Berechnung findet, aber ich will den Code weiternutzen und brauche daher absolut zuverlässige Zahlen die schon richtig sind.

Also mir wäre da ein Stück Code, das ich nicht mehr verändere, zum Beispiel zur Sicherheit in eine Datei auslagere, sicherer, als eine Datendatei auf der Platte. Und jetzt, wo Kommas, Leerzeichen und Zeilenunmbrüche in der Datei passieren, wird das ja viel konmplizierter, als Primzahlen auszurechnen.

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

int hasdiv(int cnd,int* lst)
{
	while (*lst)
		if (cnd%*lst++==0)
			return 1;
	return 0;
}

int* genprmlst(int cnt)
{
	int prmcnt=0,*lst=malloc(cnt*sizeof(int)+1),cnd=2;
	lst[0]=0;
	while (prmcnt<cnt)
	{
		while (hasdiv(cnd,lst))
			++cnd;
		lst[prmcnt++]=cnd;
		lst[prmcnt]=0;
	}
	return lst;
}

int main()
{
	int *primzahlen=genprmlst(25);

	/* Listet alle eingelesenen Primzahlen auf */
	int primecounter = 0;
	do
	{
		printf("%i, %i\n", primecounter, primzahlen[primecounter]);
		primecounter++;
	} while (primzahlen[primecounter] != 0);

	free(primzahlen);
	return 0;
}

Edit: Damit man den Code viel schneller lesen kann, habe ich nach alter C-Tradition in den Bezeichnern ein paar Vokale entfernt. Entgegen der Tradition habe ich bedeutungstragende Wörter nicht unter drei Buchstaben fallen lassen, wie man bei prmcnt gut sieht, das dürfte klassisch nicht länger als pcnt oder prmc sein. Ich hoffe, die Traditionalisten können mir verzeihen.