[ANSI C] Passwortgenerator Portierung



  • Hey Folks,

    bin gerade dabei, ein altes Projekt von mir in sauberes ANSI-C zu portieren.
    Leider scheinen mir wohl einige Fakten diesbezüglich unbekannt zu sein.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <time.h>
    #include <math.h>
    
    int main(int argc, char* argv[])
    {
    	srand ( (unsigned int)time( NULL ) );
    
    	const unsigned char nPWDlenght			= 20; // Determine password length
    
    	// Set character pool
    
    	const unsigned char numbers[11]			= {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x00};
    	const unsigned char upperCase[27]		= {0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F, 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x00};
    	const unsigned char lowerCase[27]		= {0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6A, 0x6B, 0x6C, 0x6D, 0x6E, 0x6F, 0x70, 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7A, 0x00};
    	const unsigned char notes[33]			= {0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, 0x3A, 0x3B, 0x3C, 0x3D, 0x3E, 0x3F, 0x40, 0x5B, 0x5C, 0x5D, 0x5E, 0x5F, 0x60, 0x7B, 0x7C, 0x7D, 0x7E, 0x00};
    
    	// Initialize password string
    
    	unsigned char *pPWString = NULL;	
    	pPWString = (unsigned char*) malloc (nPWDlenght+1);
    	memset (pPWString, NULL, nPWDlenght+1);
    
    	for ( int i = 0; i < nPWDlenght; i++ )
    	{
    		switch ( rand()%4 )
    		{
    		case 0:
    			{
    				pPWString[i] = numbers[rand()%10];
    				break;
    			}
    		case 1:
    			{
    				pPWString[i] = upperCase[rand()%26];
    				break;
    			}
    		case 2:
    			{
    				pPWString[i] = lowerCase[rand()%26];
    				break;
    			}
    		case 3:
    			{
    				pPWString[i] = notes[rand()%32];
    				break;
    			}
    		};
    	};
    
    	printf( "%s", pPWString );
    
    	free (pPWString);
    
    	return 0;
    }
    

    Probleme gibt es dabei meines Erachtens nach, schon mit der srand Initialisierung in Zeile 10 - der VS-Compiler haut mir daher einen Fehler raus:

    pwdgen.c(12): error C2143: Syntaxfehler: Es fehlt ';' vor 'const'
    

    Was ist falsch an der srand Initialisierung ? 😕


  • Mod

    In C (nach dem alten C89-Standard, den MSVC benutzt) duerfen Variablen nur am Blockanfang definiert werden.

    Noch ein paar Kritiken (nur quer gelesen):

    const unsigned char nPWDlenght = 20; // Determine password length

    
    Es ist ja sehr richtig, hier einen *unsigned* char zu nehmen, wenn man eine Zahl meint, aber warum char statt int? Falsch verstandene (und nicht funktionierende) Mikrooptimierung, um Speicher zu sparen?
    *  ```
    const unsigned char numbers[11]         = {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x00};
    

    Da kam sich aber jemand ganz 177t vor, als er das geschrieben hat 🙄 . Und warum unsigned char? Willst du mit den Werten rechnen?

    const char numbers[] = "0123456789";
    

    pPWString = (unsigned char*) malloc (nPWDlenght+1);

    
    Der Cast ist in C unnoetig. Wenn nPWDlenght (Das heisst uebrigens len**gth**!) sowieso eine Compilezeitkonstante (20) ist, warum dann ueberhaupt malloc?
    *  ```
    memset (pPWString, NULL, nPWDlenght+1);
    

    Auch wieder so Pseudo-Haxx0r-Code. Hier ist doch eindeutig die Zahl 0 gemeint, kein Nullzeiger!

    • Man kann's auch ueberteiben mit den consts. Const ist eher da, um in einer Schnittstelle festzulegen, welche Werte der Aufrufer als konstant ansehen kann und welche nicht. Was du mit deinen lokalen Variablen machst, interessiert eigentlich niemanden.


  • Welchem C-Standard möchtest du denn genügen?

    ANSI-C ist C89 bzw. C90 und das kennt z.B nur die Variablendefinition am Funktionsanfang (vor dem Code).
    Auch kennt es keine einzeiligen Kommentare.

    Die neueren Standards sind von der ISO.
    Visual Studio unterstützt(e) bisher nur C89 mit einigen Erweiterungen².

    Für Zeichen nimmt man char (ohne unsigned oder signed )

    Das numbers kannst du auch als

    const char numbers[]    = "0123456789";
    

    definieren. Besser lesbar, gleiches Resultat (bis auf das unsigned)

    Der Rückgabewert von malloc wird in C nicht gecastet, sollte aber überprüft werden

    ²Soll sich mit der neusten Version etwas gebessert haben.



  • ... und für die Ermittlung der Ziffern brauchst du auch gar kein Array.
    Das geht auch direkt:

    pPWString[i] = rand()%10 + '0';
    

    ... und da du ja ASCII nutzt, bei dem die Buchstaben jeweils lückenlose hintereinander liegen, kannst du das in der gleichen Art bei den Buchstaben machen.



  • Na da hab ich mir ja was vorgenommen 😃 Ich sag mal so, wenn Visual Studio 2010 nur C89 kann, versuche ich es mal damit.

    SeppJ schrieb:

    Es ist ja sehr richtig, hier einen unsigned char zu nehmen, wenn man eine Zahl meint, aber warum char statt int? Falsch verstandene (und nicht funktionierende) Mikrooptimierung, um Speicher zu sparen?

    Ja. 😃 Also entweder #pragma pack(2) oder stattdessen int ? (Ja ich weiß, Mikrooptimierung ergibt nur begrenzt Sinn bei diesem "Progrämmchen")

    SeppJ schrieb:

    Da kam sich aber jemand ganz 177t vor, als er das geschrieben hat 🙄 . Und warum unsigned char? Willst du mit den Werten rechnen?

    Lass mich 😃 Ne weiß selber nicht mehr, weshalb ich damals unsigned char genommen habe. Sah wohl "professioneller" aus. 🙄

    SeppJ schrieb:

    Der Cast ist in C unnoetig.

    Gibts da irgendein Werk in dem ich den Grund dafür Nachlesen kann ?

    SeppJ schrieb:

    Wenn nPWDlenght (Das heisst uebrigens length!) sowieso eine Compilezeitkonstante (20) ist, warum dann ueberhaupt malloc?

    Nur zu Testzwecken. Endresultat soll natürlich Variabel sein.

    SeppJ schrieb:

    Auch wieder so Pseudo-Haxx0r-Code. Hier ist doch eindeutig die Zahl 0 gemeint, kein Nullzeiger!

    Schlechte Angewohnheit da NULL in C++ einfach nur '0' ist. Grad nachgelesen, danke für den Hinweis !

    SeppJ schrieb:

    Man kann's auch ueberteiben mit den consts.

    Ist aber durchaus hilfreich - Eine irrtümliche Zuweisung wird auf die Weise durch ne Fehlermeldung quittiert, solang man nicht mit Zeigern hantiert.

    DirkB schrieb:

    ANSI-C ist C89 bzw. C90 und das kennt z.B nur die Variablendefinition am Funktionsanfang (vor dem Code).
    Auch kennt es keine einzeiligen Kommentare.

    Gut zu wissen ! Habe nie explizit in C89 programmiert. Daher das mangelnde Wissen.

    DirkB schrieb:

    Der Rückgabewert von malloc wird in C nicht gecastet, sollte aber überprüft werden

    Ok, ich schau mal was in der Referenz dazu steht.

    Mach mich jetzt erstmal ans Anpassen.



  • Der Rückgabewert von malloc ist ein Zeiger auf void .
    Und die sind in C in alle anderen Zeiger konvertierbar. Ohne cast.

    Nachlesen kannst du das in den Standards selber.

    Die kosten aber (viel) Geld. Darum begnügt man sich mit den letzten Drafts davon.
    Links dazu findest du in dem als Wichtig markierten Thread auf der Startseite von diesem Unterforum: Linkliste für Neulinge

    Da stehen manche Dinge aber nicht unbedingt so klar drin.
    Aber dafür gibt es ja auch dieses Forum 😃


  • Mod

    Da es oft falsch gemacht wird und hier womöglich auch falsch rüber gekommen ist: In C89 dürfen Variablen nur am Beginn eines Blocks deklariert werden. Eine Funktion ist auch ein Block, aber bei weitem nicht der einzige:

    int main()
    {
      int i;
      for(i = 0; i < 10; ++i)
        {
          int j;    // Vollkommen ok.
          for (j = 0; j < 10; ++ j)
            {
              puts("Blah!");
              {
                int k;  // Auch ok, obwohl zwischen Anweisungen.
              }
              puts("Blupp!");
            }
        }
      return 0;
    }
    

    (Die //-Kommentare sind es hier, die in pedantischem C89 nicht ok sind :p . Hat DirkB ja auch schon angesprochen. Aber diese Kommentare versteht wohl trotzdem jeder C89-Compiler)

    PS: Ich habe oben fälschlich geschrieben, dass man Definitionen nicht im Code haben darf. Das ist unvollständig. Korrekt ist natürlich Deklarationen.



  • Soo, hab den Code portiert und eure Tipps berücksichtigt:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <time.h>
    
    int main(int argc, char* argv[], char* envp[])
    {
    	unsigned int i	= 0;
    
    	const unsigned int nPWDlength	= 45; /* Determine password length */
    
    	const char notes[]	= "!\"#$%&'()*+,-./:;<=>?@[\]^_`{|}~"; /* Set special character pool */
    
    	char *pPWString = NULL; /* Initialize password string */
    
    	srand ( (unsigned int)time(0) );
    
    	if ( pPWString =  malloc (nPWDlength+1) ) {
    		memset (pPWString, 0, (size_t)nPWDlength+1);
    	}
    	else
    		return -1;
    
    	for ( i = 0; i < nPWDlength; i++ )
    	{
    		switch ( rand()%4 )
    		{
    		case 0:
    			{
    				/* ASCII Numbers */
    				pPWString[i] = rand()%10 + '0';
    				break;
    			}
    		case 1:
    			{
    				/* ASCII upper case letters */
    				pPWString[i] = rand()%26 + 'A';
    				break;
    			}
    		case 2:
    			{
    				/* ASCII lower case letters */
    				pPWString[i] = rand()%26 + 'a';
    				break;
    			}
    		case 3:
    			{
    				/* ASCII special characters */
    				pPWString[i] = notes[rand()%32];
    				break;
    			}
    		};
    	};
    
    	printf( "%s", pPWString );
    
    	free (pPWString);
    
    	return 0;
    }
    

    Läuft und Compiliert ohne murren 😃 Lediglich Visual Studio merkt bei

    pPWString =  malloc (nPWDlength+1)
    

    noch an:

    error : ein wert vom typ "void*" kann keiner entität vom Typ "char*" zugewiesen werden
    

    Aufgrund des weggelassenen casts.

    👍


  • Mod

    Dann compilierst du wohl im C++-Modus. Tu das nicht! Es gibt da irgendwelche Schalter, die ich nicht kenne, da ich kein MSVC benutze. Aber die kann man im Internet/Handbuch finden. Wenn ich mich recht entsinne, reicht es, wenn du deinen Quellcodedateien die Endung ".c" verpasst.

    PS: Dir ist schon klar, dass dein gesamter Code nicht großartig was anderes produziert als folgender Zweizeiler?

    for (i = 0; i < nPWDlength; i++ )
      putchar(rand() % (127 - 33) + 33)
    

    Die Gewichte der Zeichen sind ein bisschen anders, aber auch nicht sehr anders. Man kann sogar argumentieren, dass dieser Ansatz sicherer ist, da alle Zeichen gleich häufig vorkommen, im Gegensatz zu deiner Implementierung, die Ziffernzeichen stark gewichtet und Sonderzeichen schwach.



  • Ja, wenn ich die Quelldateierweiterung auf .c ändere, wirds als C89 behandelt. Der Fehler ist auch nur nen Hinweis der IDE - die geht wohl weiterhin von C++ aus.

    Werd aber mal schauen, wie ich komplett in den C Modus wechsle.

    SeppJ schrieb:

    Die Gewichte der Buchstaben sind ein klein wenig anders, aber auch nicht sehr anders.

    Sinn dahinter war es, (mit etwas Anpassung) je nach Bedarf die einzelnen Bestandteile anzupassen. Bsp: Nur Großbuchstaben + Zahlen, Nur Zahlen, etc. Die Gewichtung habe ich nicht bedacht. War die einfachste Lösung, die mir damals spontan eingefallen ist.


  • Mod

    Pyro Phoenix schrieb:

    Der Fehler ist auch nur nen Hinweis der IDE - die geht wohl weiterhin von C++ aus.

    Werd aber mal schauen, wie ich komplett in den C Modus wechsle.

    Wenn ich mich recht erinnere (wie gesagt, ich benutze kein VS, aber das Thema kommt hier öfters vor), gibt es in punkto Syntax- und Fehlerhervorhebung keinen reinen C-Modus im Visual Studio.



  • Benutze in VStudio die Projektoption "Spracherweiterungen deaktivieren" um standardkonform zu bleiben.

    char* envp[]
    

    - in kein C89

    return -1
    

    - ist ebensowenig brauchbar

    statt
    notes[rand()%32];
    robuster
    notes[rand()%strlen(notes)];
    

    - statt malloc/memset besser calloc benutzen


Anmelden zum Antworten