Sebastian, if you are still having problems after @PaulOgilvie answer, then it is most likely due to not understanding his answer. Your problem is due to buffer
being allocated but not initialized. When you call malloc
, it allocates a block of at least the size requested, and returns a pointer to the beginning address for the new block -- but does nothing with the contents of the new block -- meaning the block is full random values that just happened to be in the range of addresses for the new block.
So when you call strcat(buffer, current_char_str);
the first time and there is nothing but random garbage in buffer
and no nul-terminating character -- you do invoke Undefined Behavior. (there is no end-of-string in buffer
to be found)
To fix the error, you simply need to make buffer
an empty-string after it is allocated by setting the first character to the nul-terminating character, or use calloc
instead to allocate the block which will ensure all bytes are set to zero.
For example:
int parse_path (const char *pathname)
{
int char_index = 0, ccs_index = 0;
char current_char = pathname[char_index];
char *buffer = NULL;
char *current_char_str = NULL;
if (!(buffer = malloc (2))) {
perror ("malloc-buffer");
return 0;
}
*buffer = 0; /* make buffer empty-string, or use calloc */
...
Also do not hardcode paths or numbers (that includes the 0
and 2
, but we will let those slide for now). Hardcoding "this/is/a/path/hello"
within parse_path()
make is a rather un-useful function. Instead, make your pathname
variable your parameter so I can take any path you want to send to it...
While the whole idea of realloc
'ing 2-characters at a time is rather inefficient, you always need to realloc
with a temporary pointer rather than the pointer itself. Why? realloc
can and does fail and when it does, it returns NULL
. If you are using the pointer itself, you will overwrite your current pointer address with NULL
in the event of failure, losing the address to your existing block of memory forever creating a memory leak. Instead,
void *tmp = realloc (buffer, strlen(buffer) + 2);
if (!tmp) {
perror ("realloc-tmp");
goto alldone; /* use goto to break nested loops */
}
...
}
alldone:;
/* return something meaningful, your function is type 'int' */
}
A short example incorporating the fixes and temporary pointer would be:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int parse_path (const char *pathname)
{
int char_index = 0, ccs_index = 0;
char current_char = pathname[char_index];
char *buffer = NULL;
char *current_char_str = NULL;
if (!(buffer = malloc (2))) {
perror ("malloc-buffer");
return 0;
}
*buffer = 0; /* make buffer empty-string, or use calloc */
if (!(current_char_str = malloc (2))) {
perror ("malloc-current_char_str");
return 0;
}
while (current_char != '\0' && (int) current_char != 11) {
if (char_index == 0 && current_char == '/') {
char_index++;
current_char = pathname[char_index];
continue;
}
while (current_char != '/' && current_char != '\0') {
current_char_str[0] = current_char;
current_char_str[1] = '\0';
void *tmp = realloc (buffer, strlen(buffer) + 2);
if (!tmp) {
perror ("realloc-tmp");
goto alldone;
}
strcat(buffer, current_char_str);
char_index++;
current_char = pathname[char_index];
}
if (strlen(buffer)) {
printf("buffer(%s)\n", buffer);
current_char_str[0] = '\0';
buffer[0] = '\0';
}
if (current_char != '\0') {
char_index++;
current_char = pathname[char_index];
}
}
alldone:;
return ccs_index;
}
int main(int argc, char* argv[]) {
parse_path ("this/is/a/path/hello");
printf ("hello\n");
return 0;
}
(note: your logic is quite tortured above and you could just use a fixed buffer of PATH_MAX
size (include limits.h
) and dispense with allocating. Otherwise, you should allocate some anticipated number of characters for buffer
to begin with, like strlen (pathname)
which would ensure sufficient space for each path component without reallocating. I'd rather over-allocate by 1000-characters than screw up indexing worrying about reallocating 2-characters at a time...)
Example Use/Output
> bin\parsepath.exe
buffer(this)
buffer(is)
buffer(a)
buffer(path)
buffer(hello)
hello
A More Straight-Forward Approach Without Allocation
Simply using a buffer of PATH_MAX
size or an allocated buffer of at least strlen (pathname)
size will allow you to simply step down your string without any reallocations, e.g.
#include <stdio.h>
#include <limits.h> /* for PATH_MAX - but VS doesn't provide it, so we check */
#ifndef PATH_MAX
#define PATH_MAX 2048
#endif
void parse_path (const char *pathname)
{
const char *p = pathname;
char buffer[PATH_MAX], *b = buffer;
while (*p) {
if (*p == '/') {
if (p != pathname) {
*b = 0;
printf ("buffer (%s)\n", buffer);
b = buffer;
}
}
else
*b++ = *p;
p++;
}
if (b != buffer) {
*b = 0;
printf ("buffer (%s)\n", buffer);
}
}
int main (int argc, char* argv[]) {
char *path = argc > 1 ? argv[1] : "this/is/a/path/hello";
parse_path (path);
printf ("hello\n");
return 0;
}
Example Use/Output
> parsepath2.exe
buffer (this)
buffer (is)
buffer (a)
buffer (path)
buffer (hello)
hello
Or
> parsepath2.exe another/path/that/ends/in/a/filename
buffer (another)
buffer (path)
buffer (that)
buffer (ends)
buffer (in)
buffer (a)
buffer (filename)
hello
Now you can pass any path you would like to parse as an argument to your program and it will be parsed without having to change or recompile anything. Look things over and let me know if you have questions.