Mini - Shell, Verbesserungsvorschläge?

**Sauerkraut** · 09-10-2004, 18:28

Hallo,

ich habe eine Basis - shell programmiert, es ist erst einmal der Kern.

Habt ihr zu dem Quelltext Verbesserungsvorschläge?
Insbesondere Zeile 36 (gets() ) und die Nutzung der globalen Variablen (lieber Übergabe durch Referenz?) wären da fraglich.

Hier der Quelltext farblich hinterlegt:
http://rafb.net/paste/results/FcLIcC17.html

Oder hier in s/w:

Code:

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h> 

extern int errno;

char *command;
char *parameters[10];

#define TRUE 1

// Parameterliste löschen
void clean(void)
{
        int c;

        for (c = 0; parameters[c] != NULL; c++)
           parameters[c] = '\0';

}

void type_prompt()
{
        printf("selfshell:$ ");
}

void read_command(void)
{
 int i = 1;    
        char buf[255];
        char *token;

        // Eingabe einlesen
        gets(buf);

        // Kommando auslesen
        // token wird auf einen eigenen statischen Pufferbereich von strtok zeigen
        token = strtok(buf, " ");

        // Programm beenden?
        if ((strcmp(token, "q") == 0))
                exit(0);

        // Dieser befindet sich vermutl. im Stackbereich dieser Funktion
        // Daher Speicher für die globalen Variablen allokieren
        command = malloc(strlen(token) + 1);
        strcpy(command, token);

        // argv[0] muss der Programmname sein
        parameters[0] = malloc(strlen(token) + 1);
        parameters[0] = command;

        // Parameter auslesen
        while((token = strtok(NULL, " ")) != NULL)
        {
                parameters[i] = malloc(strlen(token) + 1);
                strcpy(parameters[i], token);
                ++i;
        }

}      
int main( void )
{
        int j;
        char buf[255];
        char *token;

   while(TRUE)
   {
        type_prompt();
        read_command();

        printf("Befehl: %s\n", command);
        printf("Argumente: ");

        for(j = 1; parameters[j] != NULL; j++)
                printf("\n\t%d: \"%s\"", j, parameters[j]);
        printf("\n");

        if(fork() != 0)
        {
                waitpid(-1, NULL, 0);
                clean();

        } else {
                execve(command, parameters, NULL);

                printf("Fehler bei exec(): %s\n", errno);
                exit(1);
        }

   }

   free(command);
   for(j = 0; j < 10; j++)
        free(parameters[j]);
}

Danke für Verbesserungsvorschläge!

MfG

**locus vivendi** · 10-10-2004, 19:10

Tschuldigung, das so harsch zu sagen, aber du solltest (vorerst) nicht versuchen eine komplette Shell zu programmieren. Zu viel Aufwand für zu wenig Nutzen. Insbesondere da es Einiges richtig zu machen gilt, für das ein Anfänger noch nicht die benötigte Erfahrung mitbringt. Oder lass es mich so sagen: Um Erfahrung zu gewinnen ist es vielleicht geeignet, aber du wirst vermutlich keine Shell bekommen die du dann auch wirklich benutzen möchtest.

Aber um zur Sache zu kommen:
"gets" ist nicht fraglich, man nimmt es einfach nicht! :-) Du musst einfach davon ausgehen das irgendwann jemand kommt und zuviele Zeichen eingibt. Selbst wenn nur du das Programm einsetzt, wirst du vielleicht vergessen das du nur "so" viele Zeichen eingeben darfst.

Meiner Meinung nach solltest du keine festen Grenzen verwenden wie z.b. die Beschränkung auf 10 Kommando "Wörter". Lege dynamisch genügend Ressourcen (Speicher) an. Willkürliche Grenzen sind einfach unschön, und gerade wenn sie so knapp bemessen sind, fallen sie dem Benutzer sehr unangenehm auf. Bei dir ist ganz besonders problematisch das diese Schleife hier...

Code:

// Parameter auslesen
        while((token = strtok(NULL, " ")) != NULL)
        {
                parameters[i] = malloc(strlen(token) + 1);
                strcpy(parameters[i], token);
                ++i;
         }

... auch überlaufen kann (und wird). Außerdem solltest du "strtok" auch nicht verwenden, aus einem ähnlichen Grund wie du schon im Code angemerkt hast. strtok liefert eine statische Variable zurück (nicht unbedingt auf einen eigenen Puffer!). Zumindest auf meinen System (GNU/linux glibc 2.2.3) sagt "man strtok" auch ganz eindeutig "Never use these functions".

Naja diese Schleife...

Code:

for(j = 1; parameters[j] != NULL; j++)
                printf("\n\t%d: \"%s\"", j, parameters[j]);

... wird auch überlaufen, weil nicht sichergestellt ist das "parameters[j]" an der korrekten Stelle "NULL" ist.

Weiterhin das hier...

Code:

for(j = 0; j < 10; j++)
        free(parameters[j]);

... wird keinen Speicher freigeben, weil du vorher schon durch "clear()" alles auf Null gesetzt hast.

Außerdem hast du bei vielen Funktionsaufrufen vergessen auf Fehler zu überpüfen.

Naja, ich will es mal dabei belassen, es ist noch mehr falsch bzw. überflüssig.
Auch wenn jetzt so einiges daran falsch ist, lass dir den Spass am Programmieren nicht verderben.

**Sauerkraut** · 04-12-2004, 14:14

Der Vollständigkeit halber hier nochmal die verbesserte, fehlerfreiere Version der mini shell.
Falls jemand Fehler entdeckt oder Verbesserungsvorschläge hat, bitte melden.

MfG

Code:

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>

#define TRUE 1

extern int errno;


void type_prompt()
{
	printf("selfshell:$ ");
}


// Speicher für ein token allokieren und es darin ablegen

void getspaceacp(char **to, char *token, int *count)
{
	if ((to[*count] = malloc(strlen(token) + 1)) == NULL) {
		printf("Fehler bei malloc(): %s\n", errno);
		exit(1);
	}
	strcpy(to[*count], token);
}

int read_command(char **command, char ***parameters)
{
	int i; // Für die Parameterliste
	char buf[255];	
	char *cpoint = NULL, *token = NULL, *newline = NULL;

	// Eingabe lesen
	if ((fgets(buf, sizeof(buf), stdin)) == NULL) {
		printf("Fehler beim Einlesen des Kommandos: %s\n", errno);
		exit(1);
	}
	
	// Keine korrekte Eingabe?
	if (strcmp(buf, "\n") == 0) {
		printf("Keine korrekte Eingabe. Ende.\n");
		exit(0);
	}

	// newline entfernen
	if ((newline = strchr(buf, '\n')) != NULL)
		*newline = '\0';

	// Eingabe (Kommando) auslesen
	token = strtok_r(buf, " ", &cpoint);

	// Programm beenden?
	if ((strcmp(token, "q") == 0))
		exit(0);
	
	else {

		i = 0;
		// Kommando abspeichern
		getspaceacp(command, token, &i); // &i hier nur "dummy"
	
		// argv[0] in der Parameterliste muss der Programmname sein; hier setzen

		*parameters = malloc(sizeof(char *)); // array hat jetzt 1 Element
		getspaceacp(*parameters, token, &i);
		i++;

		// Parameter auslesen und abspeichern
		while((token = strtok_r(NULL, " ", &cpoint)) != NULL) {
			*parameters = realloc(*parameters, (i + 1) * sizeof(char *));
			getspaceacp(*parameters, token, &i);
			i++;
		}
		(*parameters)[i] = NULL;
	}

	return i; // Anzahl der array - Elemente
}


void show_command(char **command, char ***parameters, int arraycount)
{
	int j;
		
	printf("Befehl:\t\"%s\"\n", *command);	
	printf("Parameterliste: ");

	for(j = 0; j < arraycount; j++)
		printf("\n\t%d: \"%s\"", j, (*parameters)[j]);

	printf("\n");
}

	
// Parameterliste durch Speicherfreigabe und "nullen" der Zeiger bereinigen

void clean(char **command, char ***parameters, int arraycount)
{
	int i;

	// Kommando
	free(*command);
	*command = NULL;

	// Elemente des dynamischen arrays
	for (i = arraycount - 1; i >= 0; i--) {
		free((*parameters)[i]);
		free(parameters[i]);
		parameters = NULL;
	}

	// dynamisches array

}


int main(void)
{
	int j;
	char *command = NULL;
	char **parameters = NULL; // dynamisch wachsendes array
	int arraycount = 0;	  // Anzahl der array - Elemente

	while(TRUE)
   	{
		type_prompt();
		arraycount = read_command(&command, &parameters);
		show_command(&command, &parameters, arraycount);

		if(fork() != 0)
		{
			waitpid(-1, NULL, 0);

			clean(&command, &parameters, arraycount);

		} else {
			execve(command, parameters, NULL);
		
			printf("Fehler bei exec(): %s\n", errno);
			exit(1);
		}
	}
}

**wraith** · 04-12-2004, 15:54

Dreh' mal die Warnungen bei deinem Compiler hoch (in deinem Fall wohl gcc -W -Wall -std=c99 -pedantic)
Bei ein paar printfs sind die Übergabeparameter falsch (zb. errno übergeben, aber %s als Format).

Und was fällt hier auf?

Code:

// Elemente des dynamischen arrays
for (i = arraycount - 1; i >= 0; i--) {
    free((*parameters)[i]);
    free(parameters[i]);
    parameters = NULL; //<-- soll hier wirklich parameters auf 0 zeigen?
}

Und eine kleine Stilsache bei malloc/realloc, gerade bei den vielen 'Zeiger-auf- Zeiger'.
Als Argument vom sizeof bei malloc/realloc übergibst du immer eine Dereferenzierung mehr, wie dem Zeiger, dem du das Resultat zuweist.
Aus:

Code:

*parameters = realloc(*parameters, (i + 1) * sizeof(char *));

wird dann:

Code:

*parameters = realloc(*parameters, (i + 1) * sizeof **parameters);

Das ist immer richtig.

**KL47** · 04-12-2004, 20:21

Zitat von Sauerkraut

...oder Verbesserungsvorschläge hat, bitte melden.

Als ersten Schritt könntest du mal die ganzen seltsamen .h-Header durch vernünftige, das malloc durch new und die printfs durch cout ersetzen.

Ernst: Du solltest _dringend_ was an der Einrückung und den Leerzeilen und -zeichen ändern, das erhöht die Übersichtlichkeit enorm. Bei den Klammern solltest du auch konsequenter sein: Bei den Methoden machst du sie anders als z.B. bei der for-Schleife. Und Variablen solltest du erst dann deklarieren, wenn du sie auch brauchst. Das i ("int i; // Für die Parameterliste") brauchst du z.B. erst viel später.

So, das wärs erstmal.

**Gsus** · 06-12-2004, 13:45

Hallo erstmal

erstmal ich finde diese Idee klasse ich wollte das demnächst selber in angriff nehemen ich meine das ist doch nur so zum rumtesten und daher doch ganz toll.

nebenbei kam einer meiner profesoren vorbei und hat die idee auch ganz gut gefunden er meint so kommt man schneller hinter die ganze sache als wenn man die ganze zeit nur helloworld tippt.

so ich habe mir schon das programm etwas angeschaut und habe es ein klein wenig abgeändert nichts besonderes habe erstmal

'q' durch exit 'exit' ersetzt weil ich es doch sehr irritirend fand.

bin gerde dabei die Umgebungvariablen mit einzubinden ich hoffe du hast kein problem damit

ich meine ist ja dein code (ich bin selber erst am anfang von systemnahen c deswegen spiele ich nur ein wenig rum)

okay das wars erstmal wenn ich die umgebungsvariablen drin habe poste ich den code

**Gsus** · 07-12-2004, 10:04

hallo,

ich habe jetzt mit dieser minishell etwas rum gespielt und hier und da was gemacht. da ist mir etwas aufgefallen

ich konnte ls nur ohne parameter ausführen weil sich sonst das programm mit der fehler meldung Speicherschutzverletzung beendet hat ?? der fehler ging weg als ich aus der main clean() weggemacht habe??

aber eigentlich hat clean ja schon seine berechtigung.

hier der source code wie weit ich ihn verändert habe :

Code:

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>

#define TRUE 1
#define FALSE 0

extern int errno;

void type_prompt(char *user, char *host);
void getspaceacp(char **to, char *token, int *count); // Speicher für ein token allokieren und es darin ablegen
int read_command(char **command, char ***parameters); //Liest commando in den Speicher ein
void show_command(char **command, char ***parameters, int arraycount);
void clean(char **command, char ***parameters, int arraycount);



void type_prompt(char *user,char *host)
{
	printf("%s@%s:$",user,host);
}


// Speicher für ein token allokieren und es darin ablegen

void getspaceacp(char **to, char *token, int *count)
{
	if ((to[*count] = malloc(strlen(token) + 1)) == NULL) {
		printf("Fehler bei malloc(): %s\n", errno);
		exit(1);
	}
	strcpy(to[*count], token);
}

int read_command(char **command, char ***parameters)
{
	int i; // Für die Parameterliste
	char buf[255];	
	char *cpoint = NULL, *token = NULL, *newline = NULL;

	// Eingabe lesen
	if ((fgets(buf, sizeof(buf), stdin)) == NULL) {
		printf("Fehler beim Einlesen des Kommandos: %s\n", errno);
		exit(1);
	}
	
	// Keine korrekte Eingabe?
	if (strcmp(buf, "\n") == 0) {
		printf("Keine korrekte Eingabe. Ende.\n");
		exit(0);
	}

	// newline entfernen
	if ((newline = strchr(buf, '\n')) != NULL)
		*newline = '\0';

	// Eingabe (Kommando) auslesen
	token = strtok_r(buf, " ", &cpoint);

	// Programm beenden?
	if ((strcmp(token, "exit") == 0)) 	//exit ist der Standart Beenden Befehl für eine Shell
		exit(0);
	
	else {

		i = 0;
		// Kommando abspeichern
		getspaceacp(command, token, &i); // &i hier nur "dummy"
	
		// argv[0] in der Parameterliste muss der Programmname sein; hier setzen

		*parameters = malloc(sizeof(char *)); // array hat jetzt 1 Element
		getspaceacp(*parameters, token, &i);
		i++;

		// Parameter auslesen und abspeichern
		while((token = strtok_r(NULL, " ", &cpoint)) != NULL) {
			*parameters = realloc(*parameters, (i + 1) * sizeof(char *));
			getspaceacp(*parameters, token, &i);
			i++;
		}
		(*parameters)[i] = NULL;
	}

	return i; // Anzahl der array - Elemente
}


void show_command(char **command, char ***parameters, int arraycount)
{
	int j;
		
	printf("Befehl:\t\"%s\"\n", *command);	
	printf("Parameterliste: ");

	for(j = 0; j < arraycount; j++)
		printf("\n\t%d: \"%s\"", j, (*parameters)[j]);

	printf("\n");
}

	
// Parameterliste durch Speicherfreigabe und "nullen" der Zeiger bereinigen

void clean(char **command, char ***parameters, int arraycount)
{
	int i;

	// Kommando
	free(*command);
	*command = NULL;

	// Elemente des dynamischen arrays
	for (i = arraycount - 1; i >= 0; i--) {
		free((*parameters)[i]);
		free(parameters[i]);
		parameters = NULL;
	}

	// dynamisches array

}




int main(int argc, char *argv[],char *envp[])
{

  int j;
  char *command = NULL;
  char **parameters = NULL; // dynamisch wachsendes array
  int arraycount = 0;	  // Anzahl der array - Elemente
  
  char *user = getenv("USER"), *host = getenv("HOST");  
  
    
  while(TRUE){
    
    type_prompt(user,host);
    arraycount = read_command(&command, &parameters);
    show_command(&command, &parameters, arraycount);
    
    if(fork() != 0)
    {
      waitpid(-1, NULL, 0);
      //clean(&command, &parameters, arraycount);
	
    } 
    else{
      execve(command, parameters, envp);
      
      printf("Fehler bei exec(): %s\n", errno);
      exit(1);
      
    }	
  }	
}

mfg gsus

PS: ich habe prototypen angelegt weil ich auf einen solaris arbeite und der sonst die ganze zeit warnings ausgibt wenn man das nicht macht und das nervt

**wraith** · 07-12-2004, 13:10

Zitat von Gsus

ich konnte ls nur ohne parameter ausführen weil sich sonst das programm mit der fehler meldung Speicherschutzverletzung beendet hat ?? der fehler ging weg als ich aus der main clean() weggemacht habe??

aber eigentlich hat clean ja schon seine berechtigung.

http://www.mrunix.de/forums/showpost...03&postcount=4
Die anderen Fehler sind auch noch drin.

PS: ich habe prototypen angelegt weil ich auf einen solaris arbeite und der sonst die ganze zeit warnings ausgibt wenn man das nicht macht und das nervt

Ah, da warnt er, obwohl völlig korrekt: jede Definition ist auch eine Deklaration.

**Sauerkraut** · 11-12-2004, 19:55

Hallo!

Zitat von wraith

Dreh' mal die Warnungen bei deinem Compiler hoch (in deinem Fall wohl gcc -W -Wall -std=c99 -pedantic)
Bei ein paar printfs sind die Übergabeparameter falsch (zb. errno übergeben, aber %s als Format).

Das habe ich gemacht und die Fehler korrigiert.
Allerdings frage ich mich, warum er wegen der Anweisung

Code:

// Eingabe (Kommando) auslesen
        token = strtok_r(buf, " ", &cpoint);

diesen Fehler bringt:

shell.c:57: Warnung: Zuweisung erzeugt Zeiger von Ganzzahl ohne Typkonvertierung

Kein compiler Fehler wird somit bei

Code:

  token = (char *) strtok_r(buf, " ", &cpoint);

gemeldet.

Er bringt aber auch noch den Fehler:

shell.c: In Funktion »read_command«:
shell.c:57: Warnung: implicit declaration of function `strtok_r'

Warum?

Zitat von wraith

Und was fällt hier auf?

Code:

// Elemente des dynamischen arrays
for (i = arraycount - 1; i >= 0; i--) {
    free((*parameters)[i]);
    free(parameters[i]);
    parameters = NULL; //<-- soll hier wirklich parameters auf 0 zeigen?
}

Ich lasse unbenutzte und frisch deklarierte Zeiger auf
NULL zeigen, sollte man dies nicht tun?

Gruß

Code:

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>

#define TRUE 1

extern int errno;


void type_prompt()
{
   printf("selfshell:$ ");
}


// Speicher für ein token allokieren und es darin ablegen

void getspaceacp(char **to, char *token, int *count)
{
	if ((to[*count] = malloc(strlen(token) + 1)) == NULL) 
	{
		perror("Fehler in getspaceacp() bei malloc()");
		exit(1);
	}
	strcpy(to[*count], token);
}

int read_command(char **command, char ***parameters)
{
	int i;	// Für die Parameterliste
	char buf[255];	
	char *cpoint = NULL, *token = NULL, *newline = NULL;

	// Eingabe lesen
	if ((fgets(buf, sizeof(buf), stdin)) == NULL) 
	{
		perror("Fehler in read_command() bei fgets()");
		exit(1);
	}
	
	// Keine korrekte Eingabe?
	if (strcmp(buf, "\n") == 0) 
	{
		printf("Keine korrekte Eingabe. Ende.\n");
		exit(1);
	}

	// newline entfernen
	if ((newline = strchr(buf, '\n')) != NULL)
		*newline = '\0';

	// Eingabe (Kommando) auslesen
	token = (char *) strtok_r(buf, " ", &cpoint);

	// Programm beenden?
	if ((strcmp(token, "q") == 0))
		exit(0);
	
	else {
		i = 0;

		// Kommando abspeichern
		getspaceacp(command, token, &i); // &i hier nur "dummy"
	
		//argv[0] in der Parameterliste muss der Programmname sein; hier setzen
		*parameters = malloc(sizeof(**parameters)); // array hat jetzt 1 Element
		getspaceacp(*parameters, token, &i);
		
		i = 1; // Weil Programmname nun gesetzt

		// Parameter auslesen und abspeichern
		while((token = (char *) strtok_r(NULL, " ", &cpoint)) != NULL) 
		{
			*parameters = realloc(*parameters, (i + 1) * sizeof(**parameters));
			getspaceacp(*parameters, token, &i);
			i++;
		}
		(*parameters)[i] = NULL;
	} 
	return i; // Anzahl der array - Elemente
}


void show_command(char **command, char ***parameters, int arraycount)
{
	int j;
		
	printf("Befehl:\t\"%s\"\n", *command);	
	printf("Parameterliste: ");

	for(j = 0; j < arraycount; j++)
		printf("\n\t%d: \"%s\"", j, (*parameters)[j]);

	printf("\n");
}

	
// Parameterliste durch Speicherfreigabe und "nullen" der Zeiger bereinigen

void clean(char **command, char ***parameters, int arraycount)
{
	int i;

	// Kommando
	free(*command);
	*command = NULL;

	// Elemente des dynamischen arrays
	for (i = arraycount - 1; i >= 0; i--) 
	{
		free((*parameters)[i]);
		free(parameters[i]);
		parameters = NULL;
	}
}


int main(void)
{
	char *command = NULL;
	char **parameters = NULL; // dynamisch wachsendes array
	int arraycount = 0;	  // Anzahl der array - Elemente

	while(TRUE)
   	{
		type_prompt();
		arraycount = read_command(&command, &parameters);
		show_command(&command, &parameters, arraycount);

		if(fork() != 0)
		{
			waitpid(-1, NULL, 0);

			clean(&command, &parameters, arraycount);

		} else 
		{
			execve(command, parameters, NULL);
		
			perror("Fehler in main() bei exec()"); 
			exit(1);
		}
	}
}

**Sauerkraut** · 11-12-2004, 20:00

Zitat von KL47

Als ersten Schritt könntest du mal die ganzen seltsamen .h-Header durch vernünftige, das malloc durch new und die printfs durch cout ersetzen.

Ernst: Du solltest _dringend_ was an der Einrückung und den Leerzeilen und -zeichen ändern, das erhöht die Übersichtlichkeit enorm. Bei den Klammern solltest du auch konsequenter sein: Bei den Methoden machst du sie anders als z.B. bei der for-Schleife. Und Variablen solltest du erst dann deklarieren, wenn du sie auch brauchst. Das i ("int i; // Für die Parameterliste") brauchst du z.B. erst viel später.

So, das wärs erstmal.

Ich danke dir für deine Aufmerksamkeit!
Im Eifer des Gefechtes sind mir traurigerweise diese inkonsequenten Programmiereigenheiten unterlaufen.

Variablen definiere ich grundsätzlich nach dem Funktionskopf und nicht irgendwo in der code - Wallapampa.

Gruß

**Sauerkraut** · 11-12-2004, 20:03

Zitat von Gsus

Hallo erstmal

erstmal ich finde diese Idee klasse ich wollte das demnächst selber in angriff nehemen ich meine das ist doch nur so zum rumtesten und daher doch ganz toll.

nebenbei kam einer meiner profesoren vorbei und hat die idee auch ganz gut gefunden er meint so kommt man schneller hinter die ganze sache als wenn man die ganze zeit nur helloworld tippt.

so ich habe mir schon das programm etwas angeschaut und habe es ein klein wenig abgeändert nichts besonderes habe erstmal

'q' durch exit 'exit' ersetzt weil ich es doch sehr irritirend fand.

bin gerde dabei die Umgebungvariablen mit einzubinden ich hoffe du hast kein problem damit

ich meine ist ja dein code (ich bin selber erst am anfang von systemnahen c deswegen spiele ich nur ein wenig rum)

okay das wars erstmal wenn ich die umgebungsvariablen drin habe poste ich den code

Hallo!

Du kannst gerne meinen minishell Entwurf nutzen, ich denke, ich bin hier einen unkomplizierten und verständlichen Weg gegangen.

Gruß

**wraith** · 11-12-2004, 20:08

Zitat von Sauerkraut

Code:

// Eingabe (Kommando) auslesen
        token = strtok_r(buf, " ", &cpoint);

Ja, strtok_r gehört nicht zum C Standard, und mit obigen Compilerschaltern blendet er alle Nicht-Standard-C-Prototypen aus den Standardheadern aus, dann nimmt er an, das eine nicht deklarierte Funktion int als Rückgabewert hat, und int -> char* klappt nicht.

Lange Rede, entweder du benutzt kein strtok_r (strtok reicht, du hast ja keine Threads), oder du nimmst -std=c99 als Compilerschalter weg.

Ich lasse unbenutzte und frisch deklarierte Zeiger auf
NULL zeigen, sollte man dies nicht tun?

Das kannst du machen, aber der Zeiger ist an der Stelle nicht unbenutzt.
Wenn die Schleife mehr als einen Durchlauf macht, dann wird im zweiten Durchlauf ein Null-Pointer dereferenziert -> Seg-Fault.

**Sauerkraut** · 11-12-2004, 20:10

Zitat von Gsus

hallo,
ich konnte ls nur ohne parameter ausführen weil sich sonst das programm mit der fehler meldung Speicherschutzverletzung beendet hat ?? der fehler ging weg als ich aus der main clean() weggemacht habe??

aber eigentlich hat clean ja schon seine berechtigung.

Das ist seitdem ich in der clean() Funktion ein bisschen verändert habe.
So wie sie jetzt ist, finde ich sie logisch, allerdings scheint hier der Teufel im Detail zu liegen.

Sieht jemand den Fehler in der clean() Funktion?

Code:

void clean(char **command, char ***parameters, int arraycount)
{
        int i;

        // Kommando
        free(*command);
        *command = NULL;

        // Elemente des dynamischen arrays
        for (i = arraycount - 1; i >= 0; i--)
        {
                free((*parameters)[i]);
                free(parameters[i]);
                parameters = NULL;
        }
}

hier der source code wie weit ich ihn verändert habe :

Bitte schreib zukünftig noch kurz dabei, was du geändert hast, dann brauche ich kein cmp.

Gruß

**Sauerkraut** · 11-12-2004, 20:20

Zitat von wraith

Ja, strtok_r gehört nicht zum C Standard, und mit obigen Compilerschaltern blendet er alle Nicht-Standard-C-Prototypen aus den Standardheadern aus, dann nimmt er an, das eine nicht deklarierte Funktion int als Rückgabewert hat, und int -> char* klappt nicht.

Lange Rede, entweder du benutzt kein strtok_r (strtok reicht, du hast ja keine Threads), oder du nimmst -std=c99 als Compilerschalter weg.

Danke, super Erklärung!
Auf strtok_r kam ich durch Hörensagen, wurde mir bez. Sicherheit so empfohlen...

Das kannst du machen, aber der Zeiger ist an der Stelle nicht unbenutzt.
Wenn die Schleife mehr als einen Durchlauf macht, dann wird im zweiten Durchlauf ein Null-Pointer dereferenziert -> Seg-Fault.

...und ist des Rätsels Lösung!

Hier die Korrektur:

Code:

  // Elemente des dynamischen arrays
        for (i = arraycount - 1; i >= 0; i--)
        {
                free((*parameters)[i]);
                free(parameters[i]);
        }
        parameters = NULL;

Gruß

**ExRevel** · 21-12-2004, 16:47

Wenn du dir mal die Manpage zu strtok durchliest, dann siehst du warum du sie mal besser ersetzen solltest, durch eine eigene Funktion.

"Never use this function"

Wie du siehst unter Notes hat sie noch ein paar andere gravierende Problemchen.

ciao Exi

Thema: Mini - Shell, Verbesserungsvorschläge?

Themen-Optionen

Thema bewerten

Anzeige

Mini - Shell, Verbesserungsvorschläge?

Lesezeichen

Lesezeichen

Berechtigungen