[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Lost] Neue io_get_absolute_path
+/**
+ * Zerstueckelt einen Pfad in die einzelnen Elemente
+ */
+static bool get_path_element(const char* path, int index, struct path_element* element)
Soll der Kommentar eine ausreichende Beschreibung für eine Funktion mit
diesem Prototyp sein? Ich kann mir darunter ehrlich gesagt nichts
vorstellen, ohne den Code anzuschauen. Beschreibung der Parameter und
des Rückgabewerts wäre nett - bei den folgenden Funktionen ebenso.
+{
+ bool escaped = FALSE;
+ char* pos = (char*) path;
+ char* last_element = (char*) path;
+ path_sep_t left_sep = NO_SEP;
+
+ // Pfad Zeichenweise durchgehen
+ while (TRUE) {
+ // Wenn das letzte Zeichen ein Escape-Zeichen war, muss das aktuelle
+ // nicht beruecksichtigt werden
+ if (escaped == TRUE) {
+ escaped = FALSE;
+ } else {
+ size_t cur_size = (uintptr_t) pos - (uintptr_t) last_element;
+ // Wenn das aktuelle Zeichen ein Escape ist, wird escaped auf TRUE
+ // gesetzt, damit das naechste Zeichen nicht beachtet wird
+ if (*pos == '\\') {
Darf ich mir hier ein switch wünschen?
+ escaped = TRUE;
+ }
+ // Auf Service-Seperator :/ Pruefen
+ else if ((*pos == ':') && (*(pos + 1) == '/')) {
+ // Ist das das gewuenschte Element?
+ if (index-- == 0) {
+ element->left_sep = left_sep;
+ element->right_sep = SERVICE_SEP;
+
+ // Text kopieren
+ memcpy(element->text, last_element, cur_size);
+ element->text[cur_size] = '\0';
+
+ // Wir sind am Ende angekommen
+ return TRUE;
+ }
Dieses if kommt ein paar Mal unverändert vor und Codeduplikation ist
böse. Das memcpy könnte außerdem element->text overflowen, wenn ich mich
nicht täusche und path lang genug ist.
+ // Pfad-Seperator /
+ else if (*pos == '|') {
Offensichtlich unverändert kopierter Kommentar.
+/**
+ * Erstellt aus einem Beliebigen Pfad einen absoluten
+ */
+char* io_get_absolute_path(const char* path)
Vielleicht besser einen Puffer für das Ergebnis mitübergeben?
+{
+ char* cwd = getcwd(NULL, 0);
+ // Hier wird eine Liste mit all den Pfad-Elementen erstellt
+ struct path_element* element = malloc(sizeof(struct path_element));
+ list_t* element_stack = list_create();
+
+ int i = 0;
+ // Elemente einzeln holen, und an Liste anhaengen
+ while (get_path_element(path, i++, element) == TRUE) {
+ list_push(element_stack, element);
+ element = malloc(sizeof(struct path_element));
+ }
+ // Das letzte Element ist unbenutzt und kann freigegeben werden
+ free(element);
Das Verfahren funktioniert zwar, ist aber natürlich völlig ineffizient.
Du nimmt den Pfad hier in O(n²) auseinander, indem du für jeden
Bestandteil den kompletten String neu parst.
+ // Jetzt werden relative Pfade aufgeloest
+ int last_element = list_size(element_stack) - 1;
+ element = list_get_element_at(element_stack, last_element++);
Das ist verwirrend. Wozu erst eins abziehen und in der nächsten Zeile
wieder ein verstecktes ++ drauf? Außerdem wäre ein Hinweis gut, daß
last_element logisch das erste (vorderste) Element bezeichnet.
+
+ // Wenn der Pfad relativ ist wird noch cwd vorne angehaengt
+ if ((element->left_sep == NO_SEP) && (element->right_sep !=
+ SERVICE_SEP))
+ {
+ // CWD-Elemente holen und an Liste anhaengen
+ i = 0;
+ struct path_element* cwd_element = malloc(sizeof(struct path_element));
+ while (get_path_element(cwd, i++, cwd_element) == TRUE) {
+ list_insert(element_stack, last_element, cwd_element);
+ cwd_element = malloc(sizeof(struct path_element));
+ }
+ // Das letzte Element ist unbenutzt und kann freigegeben werden
+ free(cwd_element);
Das malloc/free beim letzten Durchlauf sollte man durch Umdrehen der
beiden Schleifenzeilen vermeiden können. Oben genauso.
+ // Beim letzten Element muss noch der Seperator angepasst werden
+ // TODO: Stimmt das?
+ cwd_element = list_get_element_at(element_stack, last_element);
+ cwd_element->right_sep = PATH_SEP;
+ }
+
+ // Wen der Pfad relativ zum aktuellen Service ist wird die Sache etwas
+ // komplizierter. Dann muessen von CWD erst alle verzeichnisse bis zum
+ // hintersten Service abgetrennt werden
+ if (element->left_sep == PATH_SEP)
+ {
+ list_t* cwd_list = list_create();
+
+ // CWD-Elemente holen und an Liste anhaengen
+ i = 0;
+ struct path_element* cwd_element = malloc(sizeof(struct path_element));
+ while (get_path_element(cwd, i++, cwd_element) == TRUE) {
+ cwd_list = list_push(cwd_list, cwd_element);
+ cwd_element = malloc(sizeof(struct path_element));
+ }
+ // Das letzte Element ist unbenutzt und kann freigegeben werden
+ free(cwd_element);
+
+ // Jetzt werden alle Elemente bis zum Service-Element verworfen
+ i = 0;
+ while ((cwd_element = list_get_element_at(cwd_list, i))) {
+ if (cwd_element->right_sep != SERVICE_SEP) {
+ list_remove(cwd_list, i);
> + } else {
> + break;
> + }
> + }
Da fehlt ein free. Wäre es übrigens nicht schlauer, solche Elemente erst
gar nicht in die Liste zu schreiben und dadurch mallocs zu sparen?
Das "else break" sieht außerdem irgendwie so aus, als sollte die
if-Bedingung eigentlich Teil der while-Bedingung sein. Macht aber nichts
weiter, da die Schleife ja sowieso komplett rausfliegt. ;-)
+
+ // Die uebrig gebliebenen Elemente werden angehaengt
+ i = list_size(cwd_list);
+ while ((cwd_element = list_get_element_at(cwd_list, --i))) {
+ list_insert(element_stack, last_element, cwd_element);
+ }
+
+ list_destroy(cwd_list);
+ }
+
+
+ // Dot und Dotdot Elemente eliminieren
+ i = 0;
+ int dotdot = 0;
+ while ((element = list_get_element_at(element_stack, i))) {
+ // Bei Dotdot wird das aktuelle geloescht und dotdot inkrementiert,
+ // damit die naechsten Elemente entsprechend geloescht werden
+ if (strcmp(element->text, "..") == 0) {
+ list_remove(element_stack, i);
+ dotdot++;
+ }
+ // Dot-Elemente und leere werden einfach verworfen
+ else if ((strcmp(element->text, ".") == 0) || (strlen(element->text)
+ == 0))
+ {
+ list_remove(element_stack, i);
+ }
+ // Alles andere bleibt gleich
+ else {
+ // Wenn Dotdot-Elemente da waren, muss das Element geloescht werden
+ // und dotdot dekrementiert
+ if (dotdot != 0) {
+ list_remove(element_stack, i);
+ dotdot--;
+ } else {
+ i++;
+ }
+ }
+ }
Das schreit nach einer ausgelagerten Funktion, meinetwegen static
inline. Außerdem fehlen mal wieder ein paar free.
+ // Berechnen, wieviel Speicher der fertige Pfad belegt
+ size_t size = 0;
+ i = 0;
+ while ((element = list_get_element_at(element_stack, i++))) {
+ size += strlen(element->text);
+ // Berechnen, wieviel Speicher der Seperator belegen wird
+ switch (element->right_sep) {
+ case SERVICE_SEP:
+ size += 1;
+ case PATH_SEP:
+ case PIPE_SEP:
+ size += 1;
+ }
Bitte size += 2; break; (oder noch klarer: size += strlen(":/"), das
wird sowieso wieder wegoptimiert) und hinter das size += 1 auch ein
break, falls das switch mal erweitert wird.
Dieser Code ist erstens zu trivial, um ein fall-through zu
rechtfertigen, zweitens steckt auch keine Logik, das für SERVICE_SEP in
zwei Schritte aufzutrennen und drittens _MUSS_ bei sowas ein Kommentar
/* fall-through */ oder ähnliches hin.
Das Codestück würde auch eine schön abgeschlossene Einheit für eine
Funktion bilden.
+ }
+
+ // Jezt ist es soweit, der neue Pfad kann kopiert werden
+ char* new_path = malloc(size + 1);
+ char* pos = new_path;
+ i = list_size(element_stack);
+ while ((element = list_get_element_at(element_stack, --i))) {
+ // Element-Text kopieren
+ size = strlen(element->text);
+ memcpy(pos, element->text, size);
+ pos += size;
+
+ // Seperator einfuegen
+ switch (element->right_sep) {
+ case SERVICE_SEP:
+ *(pos++) = ':';
+ *(pos++) = '/';
+ break;
+
+ case PATH_SEP:
+ *(pos++) = '/';
+ break;
+
+ case PIPE_SEP:
+ *(pos++) = '|';
+ break;
+ }
free(element);
Und wieder ein schöner Umfang für eine eigene Funktion.
+ }
+ *pos = '\0';
+ //printf("Path='%s' => '%s'\n", path, new_path);
+ free(cwd);
+ list_destroy(element_stack);
+ return new_path;
+}
+
------------------------------------------------------------------------
_______________________________________________
Lost mailing list
Lost@xxxxxxxxxxxxxxxx
http://famkaufmann.info/mailman/listinfo/lost