subpath

hi leute,

kann man folgende funktion noch effektiver coden? mir gefaellt dieses strtok nicht:/ ich finde der code ist schwer lesbar...was meint ihr?

beispiel:

input = "/TEST1/TEST2/TEST3", 1
output = "/TEST2"

input = "/TEST1/TEST2/TEST3", 0
output = "/"

input = "/TEST1/TEST2/TEST3", 3
output = "/TEST1/TEST2/TEST3"

usw

code:

#define PATH_MAX 255

static char *get_subpath(const char *dir, unsigned int depth)
{
    char *dir_copy;
    char *buff;
    char *p;

    dir_copy = (char*)malloc(PATH_MAX);
    buff = (char*)malloc(PATH_MAX);
    strcpy(dir_copy, dir);
    memset(buff, 0, sizeof(char)*PATH_MAX);

    if (depth)
        depth--;
    else {
	strcpy (dir_copy, "/");
	free( buff );
        return dir_copy;
    }

    strncpy (buff, dir_copy, sizeof(char)*PATH_MAX);
    p = strtok (buff, "/");
    while (depth--)
        p = strtok (0, "/");

    memset(dir_copy, 0, sizeof(char)*PATH_MAX);
    strncpy (dir_copy, p, sizeof(char)*PATH_MAX);
    free( buff );

    return dir_copy;
}

proggingmania

Da du ja mit #define PATH_MAX 255 die maximale Länge
des Rückgabewertes festgelegt hast, kannst du dir im
grunde genommen auch eine dynamische Speicherreservierung sparen.
Und wozu eigentlich static ?

Du könntest ja den Pfad in einem Array als Parameter übergeben:
*void get_subpath( const char dir, unsigned int depth, char sub_path[256] );

pale dog

...und diese ganzen 'sizeof(char)' kannste auch weglassen.

Shade Of Mine

Der Ansatz sieht generell sehr komisch aus. Ein paar Punkte wurden schon genannt.

Ich verstehe die Aufgabenstellung aber nicht ganz. Kannst du nochmal genauer beschreiben was du genau machen willst. Mir kommen die Zahlen naemlich gerade sehr komisch vor.

Es sind aber generell viel zuviele unnoetigen allokationen.

Statt strtok kannst du mit strchr gut durch den String gehen und dann nur einmal am Ende allokieren und kopieren. Und NIE NIE NIE in einer Funktion allokierten Speicher nach aussen geben.

pale dog

Shade Of Mine schrieb:

Und NIE NIE NIE in einer Funktion allokierten Speicher nach aussen geben.

Shade Of Mine

pale dog schrieb:

Shade Of Mine schrieb:

Und NIE NIE NIE in einer Funktion allokierten Speicher nach aussen geben.

char* gimme_hello_world() {
  char* p=malloc(100);
  strcpy(p, "hallo welt");
  return p;
}

int main() {
  pintf("%s\n", gimme_hello_world());
  return 0;
}

sieht auf den 1. Blick ganz OK aus.
DOCH wir haben ein Speicher Leck. Und zwar komischerweise in main() obwohl main() doch keine einziges alloc macht.

Wenn Zeiger hin und her gegeben werden, muss man immer achten wer fuer das free'en zustaendig ist. Deshalb macht man es meistens so, dass der der den Speicher allokiert ihn auch loeschen muss.

void gimme_hello_world(char* p) {
  strcpy(p, "hallo welt");
}

int main() {
  char* p=malloc(100);
  /*oder:
  char p[100];
  */
  gimme_hello_world(p);
  pintf("%s\n", p);
  return 0;
}

man erkennt ob ein free() fehlt deutlich leichter. weil ich habe ein malloc ohne free da stehen.

der Nachteil ist die umstaendlichere Syntax (gimme_hello_world in einer eigenen zeile) doch dem kann man abhilfe schaffen:

char* gimme_hello_world(char* p) {
  return strcpy(p, "hallo welt");
}

int main() {
  char* p=malloc(100);
  pintf("%s\n", gimme_hello_world(p));
  free(p);
  return 0;
}

Wir haben so auch die Moeglichkeit Speicher zu nutzen den gimme_hello_world nicht benutzen kann, wie zB indem wir einen eigenen Memory Manager benutzen oder einen GC zwischen schalten oder eben p auf den stack legen, etc.

Nachtrag:
Ausnahmen bestaetigen die Regel

proggingmania schrieb:

Da du ja mit #define PATH_MAX 255 die maximale Länge
des Rückgabewertes festgelegt hast, kannst du dir im
grunde genommen auch eine dynamische Speicherreservierung sparen.
Und wozu eigentlich static ?

Du könntest ja den Pfad in einem Array als Parameter übergeben:
*void get_subpath( const char dir, unsigned int depth, char sub_path[256] );

du meinst die subpath ins array rein tun?

sub_path[0] = "/"
sub_path[1] = "TEST1"
sub_path[2] = "TEST2"
sub_path[3] = "TEST3"

und dann je nach depth den path zusammenbauen? das klingt viel besser! also zuerst path aufsplitten mit split_path und dann get_subpath baut mir den path in abhaengigkeit von depth zusammen!

hi,
kann mir jemand den fehler finden? in der while schleife....
moechte das string array so befuellen:
sub_path[0] = "/"
sub_path[1] = "TEST1"
sub_path[2] = "TEST2"
sub_path[3] = "TEST3"
sub_path[3] = "TEST4"

#define PATH_MAX 255
#define NAME_MAX 255

void splitpath(const char *dir, char sub_path[PATH_MAX][NAME_MAX])
{
	unsigned int i = 0;
	char dir_tmp[PATH_MAX];
	char *p = NULL;

	strcpy(dir_tmp, dir);
	p = strchr(dir_tmp, '/');
	while(p != NULL)
	{
		strncpy(sub_path[i++], dir_tmp, p-dir_tmp+1);  <---hier ist etwas falsch
		p = strchr(p+1, '/'); <---hier ist etwas falsch
	}  
}

void get_subpath(char *sub_path, unsigned int depth, char split_path[PATH_MAX][NAME_MAX])
{
	unsigned int i = 0;

	for(i = 0; i < depth; i++) {
		strcat(sub_path, split_path[i]);
		if((i != 0) || (i != depth - 1))
			strcat(sub_path, "/");
	}
}

int main()
{
	char dir[PATH_MAX] = "/TEST1/TEST2/TEST3/TEST4";
	char split_path[PATH_MAX][NAME_MAX];
	char sub_path[PATH_MAX];

	splitpath(dir, split_path);
	get_subpath(sub_path, 2, split_path);

	return 0;
}

proggingmania

String schrieb:

...
du meinst die subpath ins array rein tun?
...

Nönööö, so habe ich das nicht gemeint, ich würde das prev_dir in sub_path[256] speichern.
*
void get_subpath( const char dir, unsigned int depth, char sub_path[256] );

pale dog

Shade Of Mine schrieb:

pale dog schrieb:

Shade Of Mine schrieb:

Und NIE NIE NIE in einer Funktion allokierten Speicher nach aussen geben.

...
Nachtrag:
Ausnahmen bestaetigen die Regel

ja, das klingt schon besser als 'NIE NIE NIE'