Disclaimer: Dieser Thread wurde aus dem alten Forum importiert. Daher werden eventuell nicht alle Formatierungen richtig angezeigt. Der ursprüngliche Thread beginnt im zweiten Post dieses Threads.
Miniklausur 8.12.2010, Aufgabe 2 Lösungsversuch
Hallo,
Ich habe mich mal an dieser Programmieraufgabe versucht, bin mir jedoch sicher, dass einiges nicht ganz passt, z.B. weil ich malloc verwende, obwohl es laut Aufgabenstellung nicht notwendig ist.
Würde jemand mal einen Blick darauf werfen?
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
int execute(const char *line) {
int argcounter = 0;
char* delimiters = " \t";
char **command = malloc(sizeof(char*) * strlen(line));
if (command == NULL)
return 1;
char *pointer = line;
while((pointer = strtok(pointer, delimiters)) != NULL) {
command[argcounter++] = pointer;
pointer = NULL;
}
pid_t pid = fork();
if (pid < 0) {
return 1;
} else if (pid == 0) {
execvp(command[0], command);
return 1;
} else {
int childStatus;
if (waitpid(pid, & childStatus, 0) < 0) {
free(command);
return 2;
} else {
free(command);
return WEXITSTATUS(childStatus);
}
}
}Gemeint ist vermutlich, dass du dir überlegen kannst, dass es nie mehr als [m]strlen(line) / 2[/m] Argumente (oder waren es [m]strlen(line) / 2 + 1[/m]? Kleine Denksportaufgabe…) werden können und du dir deswegen das benötigte Array auch auf dem Stack anlegen kannst:
char *command[strlen(line) / 2];Achtung: Wenn [m]execvp(2)[/m] zurück kehrt (z.B. wegen eines Fehlers) gibst du im geforkten Prozess [m]1[/m] an den Aufrufer zurück und hast damit zwei Kontrollflüsse, die an den Aufrufer zurückkehren werden. Hier brauchst du unbedingt [m]exit(EXIT_FAILURE);[/m] (damit der Code-Teil mit dem [m]waitpid(2)[/m]) weiter läuft. Diesen häufig gemachten Fehler schreibst du dir am besten gleich hinter die Ohren. (Ohne das [m]exit(2)[/m] verlierst du übrigens in dieser Fassung auch den Speicher auf den [m]command[/m] zeigt.) 
Das [m]free(3)[/m] im [m]else[/m]-Teil könnte früher stehen – du brauchst den Speicher ja nicht mehr, es lohnt sich also nicht, ihn bis hinters [m]waitpid(2)[/m] aufzuheben, nur um in denn in beiden Fällen freizugeben.
[m]malloc(3)[/m] sollte man meist vermeiden, da es langsamer als Stack-Allokation ist. Außnahmen sind dynamische Anforderungen ([m]realloc(3)[/m]) und große Datenmengen (da sonst der Stack überläuft).
Ein weiterer Vorteil von Stack-Allokationen ist, dass der Compiler den Speicher selbst wieder freigibt. Besonders bei Klausuren ist das praktisch, da man sich nicht um [m]free(3)[/m] kümmern muss; aber es macht natürlich auch die Programme besser lesbar.
Hier sollte -1 zurückgegeben werden.
Diese Zeile brauchst du nicht, da ja zu Beginn jedes Schleifendurchlaufs [m]pointer[/m] neu gesetzt wird.
[m]command[/m] muss NULL-terminiert sein, das ist hier nicht der Fall. Du kannst die Schleife etwas vereinfachen, dann wird das automatisch für dich erledigt:
command[argcounter++] = strtok(line, delimiters);
while ((command[argcounter++] = strtok(NULL, delimiters)) != NULL)
;Hier fehlt ein [m]free(3)[/m].
Hier sollte -1 zurückgegeben werden.
Wie neverpanic schon schrieb, das sollte [m]exit()[/m] sein.
Hier sollte -1 zurückgegeben werden. Das ist ein normaler Fehler und hat nichts damit zu tun, wie sich das Programm beendet hat.
Hier musst du zuerst prüfen ob sich das Programm selbst beendet hat ([m]WIFEXITED[/m]; es könnte ja auch ein Signal aufgetreten sein) und dann mit [m]WEXITSTATUS[/m] den Exit-Code zurückgeben, sonst soll -2 zurückgegeben werden.
Du darfst [m]WEXITSTATUS[/m] nur dann verwenden, wenn du zuerst mit [m]WIFEXITED[/m] geprüft hast, ob das Kind überhaupt einen Exitstatus hat.
Edit: Fehler bei [m]waitpid(3)[/m] korrigiert.
Vielen Dank für die ausführliche Korrektur. Bei den return-Werten muss ich die Angabe also jeweils als “minus” interpretieren? Ich habe die “-” für Aufzählungsstriche gehalten.
Hier nun eine (hoffentlich) richtige Version der Funktion execute, für alle, die die Aufgabe noch in Zukunft zur Übung bearbeiten:
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
int execute(const char *line) {
int argcounter = 0;
char* delimiters = " \t";
char *command[2+strlen(line)/2];
command[argcounter++] = strtok(line, delimiters);
while((command[argcounter++] = strtok(NULL, delimiters)) != NULL);
pid_t pid = fork();
if (pid < 0) {
return -1;
} else if (pid == 0) {
execvp(command[0], command);
exit(EXIT_FAILURE);
} else {
int childStatus;
if (waitpid(pid, & childStatus, 0) < 0) {
return -1;
} else {
if (WIFEXITED(childStatus) != 0) {
return WEXITSTATUS(childStatus);
} else {
return -2;
}
}
}
}Und ob du wirklich richtig stehst… siehst du wenn das Licht angeht… (und es geht auf der Bahn nebenan an, sorry):
Nehmen wir an [m]line[/m] besteht aus den Zeichen [m]„a b c d\0“[/m], dann würde [m]strlen(line)[/m] 7 zurückgeben (weil die Länge immer ohne das 0-Byte am Ende gezählt wird). In diesem Fall würdest du also ein Array der Länge 7/2 = 3 (Ganzzahl-Division!) anlegen. Du hast aber 4 Parameter und brauchst noch Platz für das [m]NULL[/m] hinter dem letzten Parameter. Korrekt muss es also [m]strlen(line) / 2 + 2[/m] (oder [m](strlen(line) + 1) / 2 + 1[/m]) heißen.
Das Umformen üben wir aber nochmal, oder? 
Edit: Ich vermute das soll wieder eine „Denksportaufgabe“ sein?
Stimmt, an das NULL habe ich nicht gedacht. Ich editiere mal +2 rein, dann sollte es ja auf jeden Fall groß genug sein.
Ja. Es ist (oft) Konvention, negative Werte für die Fehlerfälle zu verwenden. Außerdem steht im Text auch -2. Und da die Exit-Codes positiv sind, könntest du sonst Fehler- von Erfolgsfällen nicht unterscheiden.
Diese Zeile brauchst du nicht, das macht die Schleife schon für dich, da die Zuweisung immer ausgeführt wird.
Wie du schon richtig festgestellt hast sind die beiden Ausdrücke nicht äquivalent, der eine ist also auch keine Umformung des anderen.
Betrachten wir das ganze für verschiedene Strings:
- [m]„a b\0“[/m], mit der ersten Formel gäbe das ein Array der Länge 3, mit der zweiten ebenfalls, in beiden Fällen genug Platz.
- [m]„a b \0“[/m], mit der ersten Formel 4, mit der zweiten 3. Hier sind es aber nur zwei Parameter (plus der Platz für [m]NULL[/m]) Arraylänge 3 reicht also.
Fazit: Beide Formeln funktionieren, mit der zweiten spart man u.U. [m]sizeof(void *)[/m] an Platz ein, dafür ist sie ein bisschen komplizierter.
[quote=neverpanic]
Wie du schon richtig festgestellt hast sind die beiden Ausdrücke nicht äquivalent, der eine ist also auch keine Umformung des anderen.[/quote]
Ja sowas in der Art dachte ich mir schon. So früh auf etwas zu antworten ohne die Aufgabenstellung gelesen zu haben kann aus Langeweile mal passieren. 
Disclaimer: Ich habe die Aufgabenstellung auch nicht gelesen 
1 Like