Wrap C block in function. Beginner question

Question

I found the C snippet to get the current working directory from here. Essentially, the code is:

char directory[_MAX_PATH];
getcwd(directory, sizeof(directory))

I want to abstract that into another function, in a different file (so it can be swapped out on different platforms if necessary).

Currently, I have in the external file

void getCurrentDirectory(char *directory) {
    getcwd(directory, sizeof(directory));
}

and in the main file

char directory[100];
getCurrentDirectory(directory);
printf("%s", *directory);

However, when printing to screen, I get nonsense (possibly trying to print memory location as a string?)

I'm sure it's something blindingly obvious to a non-beginner. What's going on?

Edit: I'm on Windows 7, btw

Thanks.

Answer 1

Theres a number of things that you are doing wrong here:

void getCurrentDirectory(char *directory) 
  {
      getcwd(directory, sizeof(directory));
  }

Mistake 1:

`sizeof(directory)` 

gives you size of a pointer, to be precise, char *. Your intention is to pass size of the array, and not the pointer size.

Mistake 2:

`printf("%s", *directory);` 

Passes first element of the array to the printf not the address of the array. Your intention is to print the entire array not just the first element.

Corrected Solution

You should be doing

void getCurrentDirectory(char *directory, size_t arrSize)  
{                                         ^^^^^^^^^^^^^^
    getcwd(directory, arrSize);
}

Size of the array is passed explicitly so the function can just use it.

In the main while printing the contents of the array:

   printf("%s", directory);

Answer 2

This line: printf("%s", *directory);

should be: printf("%s", directory);

You're passing in the first element (directory[0]) to the printf, not the pointer to the char array.

Answer 3

If it's C++ I'd suggest using boost::filesystem if at all possible, which hides all of the underlying platform details and gives you C++ style interface instead of the buffer overflow prone C functions.

Answer 4

You are passing the size of a char* to getcwd, instead of the size of the array.

Pass a size parameter to your function.

void getCurrentDirectory(char *directory, size_t size) {
    getcwd(directory, size);
}

and then:

char directory[100];
getCurrentDirectory(directory, sizeof(directory));
printf("%s", *directory);

Also, if you're using Windows, you should probably change your array size to the predefined MAX_PATH to avoid a potential buffer overflow. getcwd takes a length, but I don't think all of the file functions do.

Answer 5

You should be allocating the buffer locally (where the necessary length is known, and the actual length needs to be known) and returning a string:

std::string
getCurrentDirectory()
{
    char results[_MAX_PATH];
    if ( getcwd( results, sizeof(results) ) == NULL )
        throw std::ios_base::failure( "Could not get current directory" );
    return std::string( results );
}

Note too that _MAX_PATH is only a guess; the actual maximum is not a compile time constant (since it depends on the file system). An implementation which takes this into account might look something like:

std::string
getCurrentDirectory()
{
    long length = pathconf( ".", _PC_PATH_MAX );
    if ( length == -1 )
        throw std::ios_base::failure(
                "Could not determine necessary buffer length to get current directory" );
    std::string results( length, '\0' );
    if ( getcwd( &results[0], results.size() ) == NULL )
        throw std::ios_base::failure( "Could not get current directory" );
    results.resize( strlen( results.c_str() );
    return results;
}

This is probably overkill, however, if the program is only going to be used on a personal system with no NFS or SMB mounted drives.

Answer 6

Since it is C++, why not do this:

std::string getCurrentDirectory()
{
    char directory[_MAX_PATH] = {};
    getcwd(directory, sizeof(directory));
    return directory;
}

Answer 7

You cannot find out the size of a memory block pointed at by a pointer using sizeof like that. It will evaluate to the size of the pointer itself.

Change your function to:

void getCurrentDirectory(char *directory, size_t buf_max)
{
    getcwd(directory, buf_max);
}

Answer 8

Now to answer what you've asked:

When getcwd fails for some reason the contents of the array pointed to by directory (in your case) is undefined. Therefore, with your buggy implementation you will see junk in most cases. (Also, you should check return value from getcwd, it returns -1 when it fails)

Now, the reason for failure in your case is the size that you are specifying using sizeof(directory) is just the size of a pointer (which would likely to be 4) and characters in the name of current working directory you are trying to print are more than that. This will work fine for directory of size 3 or less.

And, finally many others here have already explained you how to possibly fix it.