4

Sorry to keep hammering on this, but I'm trying to learn :). Is this any good? And yes, I care about memory leaks. I can't find a decent way of preallocating the char*, because there simply seems to be no cross-platform way.

const string getcwd()
{
    char* a_cwd = getcwd(NULL,0);
    string s_cwd(a_cwd);
    free(a_cwd);
    return s_cwd;
}

UPDATE2: without Boost or Qt, the most common stuff can get long-winded (see accepted answer)

rubenvb
  • 74,642
  • 33
  • 187
  • 332
  • See rubenvb's [previous question](http://stackoverflow.com/questions/2868680/what-is-a-cross-platform-way-to-get-the-current-directory) on this. – Matthew Flaschen May 19 '10 at 21:46
  • What is `CAKE_getcwd`? Google finds me nothing. Plain old `getcwd` *won't* work as you hope on Solaris. Although it will allocate memory when you pass it a null pointer, the amount of memory it allocates is based entirely on the second `getcwd` parameter, not the length of the string it wants to return. So, in your example, it would allocate *zero* bytes (or simply fail with EINVAL). – Rob Kennedy May 20 '10 at 02:10
  • CAKE_getcwd was my workaround define for the deprecation warning of plain getcwd vs _getcwd on Windows. Fixed – rubenvb May 20 '10 at 09:25
  • Personally, I prefer more informative titles than just "A question on *subject*".. we know it's a question a we know what topic it's because of the tags, so specifics are nice :) – Paul May 20 '10 at 18:11
  • Fixed title and will try to be more descriptive in the future – rubenvb May 21 '10 at 15:53

7 Answers7

8

If you want to remain standard, getcwd isn't required to do anything if you pass to it a NULL; you should instead allocate on the stack a buffer that is "large enough" for most occasions (say, 255 characters), but be prepared for the occasion in which getcwd may fail with errno==ERANGE; in that case you should allocate dinamically a bigger buffer, and increase its size if necessary.

Something like this could work (notice: not tested, just written by scratch, can be surely improved):

string getcwd()
{
    const size_t chunkSize=255;
    const int maxChunks=10240; // 2550 KiBs of current path are more than enough

    char stackBuffer[chunkSize]; // Stack buffer for the "normal" case
    if(getcwd(stackBuffer,sizeof(stackBuffer))!=NULL)
        return stackBuffer;
    if(errno!=ERANGE)
    {
        // It's not ERANGE, so we don't know how to handle it
        throw std::runtime_error("Cannot determine the current path.");
        // Of course you may choose a different error reporting method
    }
    // Ok, the stack buffer isn't long enough; fallback to heap allocation
    for(int chunks=2; chunks<maxChunks ; chunks++)
    {
        // With boost use scoped_ptr; in C++0x, use unique_ptr
        // If you want to be less C++ but more efficient you may want to use realloc
        std::auto_ptr<char> cwd(new char[chunkSize*chunks]); 
        if(getcwd(cwd.get(),chunkSize*chunks)!=NULL)
            return cwd.get();
        if(errno!=ERANGE)
        {
            // It's not ERANGE, so we don't know how to handle it
            throw std::runtime_error("Cannot determine the current path.");
            // Of course you may choose a different error reporting method
        }   
    }
    throw std::runtime_error("Cannot determine the current path; the path is apparently unreasonably long");
}

By the way, in your code there's a very wrong thing: you are trying to dellocate a_cwd (which presumably, in the nonstandard extension, is allocated with malloc or with some other memory allocation function, since getcwd is thought for C) with delete: you absolutely shouldn't do that, keep in mind that each allocation method has its deallocation counterpart, and they must not be mismatched.

Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
  • Two things: your first if is missing a ')' at the end, and about scoped_ptr: it won't be in c++0x, right? Is there any noticable impact of scoped_ptr vs auto_ptr in this case? Thanks – rubenvb May 21 '10 at 16:02
  • Would it be better to use c++0x/tr1 shared_ptr or would this make absolutely no difference? – rubenvb May 21 '10 at 17:47
  • In C++0x you should use unique_ptr, shared_ptr is overkill. For the missing ')', thank you, fixed. – Matteo Italia May 21 '10 at 20:27
3

This will work on Windows and Linux, since they both support the automatic allocation behavior when the buf argument to getcwd is NULL. However, be aware that this behavior is not standard, so you may have issues on more esoteric platforms.

You can do it without relying on this behavior, though:

const string getcwd()
{
    size_t buf_size = 1024;
    char* buf = NULL;
    char* r_buf;

    do {
      buf = static_cast<char*>(realloc(buf, buf_size));
      r_buf = getcwd(buf, buf_size);
      if (!r_buf) {
        if (errno == ERANGE) {
          buf_size *= 2;
        } else {
          free(buf);
          throw std::runtime_error(); 
          // Or some other error handling code
        }
      }
    } while (!r_buf);

    string str(buf);
    free(buf);
    return str;
}

The above code starts with a buffer size of 1024, and then, if getcwd complains that the buffer is too small, it doubles the size and tries again, and repeats until it has a large enough buffer and succeeds.

Note that calling realloc with its first argument as NULL is identical to malloc.

Tyler McHenry
  • 74,820
  • 18
  • 121
  • 166
  • If you want to do it so that it conforms to POSIX standards and handles any arbitrary path length, this is what you have to do. If you want to make it shorter, you have to give up one of those two requirements. – Tyler McHenry May 19 '10 at 21:53
  • OK, this is where I toss POSIX overboard :) – rubenvb May 19 '10 at 22:05
  • 2
    You're free to do so, but since the emphasis of your question seemed to be on cross-platform portability, I would have expected you to put up with the extra dozen or so lines of code... – Tyler McHenry May 19 '10 at 22:29
  • Well, it may be a bit long, but one, you don't have to look at it, just call it, and two, unless the buffer isn't large enough, it should be virtually as efficient as the shorter version (and given that there's an I/O hit to get the actual directory, then the difference really shouldn't matter). Since you can basically write this and forget about it, I'd definitely say to go for the POSIX-compliant version. – Jonathan M Davis May 19 '10 at 23:20
  • I'm getting a "invalid conversion from void* to char*" on the realloc line with GCC 4.4... – rubenvb May 20 '10 at 18:23
  • 3
    +1, but to be really compliant, `buf = realloc(buf, ...);` is bad, because if `realloc()` fails, you get a memory leak. So to be *really* pedantic, you should have something like `tmp = realloc(buf, ...); if (tmp) buf = tmp; else { /* handle error */ }` – Alok Singhal May 20 '10 at 18:37
  • That's unnecessarily long and it's not exception safe. – JoeG May 21 '10 at 08:50
  • Matteo Italia's answer below is actually, in my opinion, the most correct answer in terms of, efficiency, error handling, and exception safety. It's even longer than mine, though. :) – Tyler McHenry May 21 '10 at 15:45
  • As mentioned by others, realloc() may return a NULL pointer. It has to be checked. – Alexis Wilke Oct 27 '12 at 03:11
3

You must not pass a null pointer to the constructor of a std::string, so you must check the buffer pointer getcwd() returns isn't null. Also, the buffer pointer you pass to getcwd() must not be null.

std::string getcwd() {
    char buf[FILENAME_MAX];
    char* succ = getcwd(buf, FILENAME_MAX);
    if( succ ) return std::string(succ);
    return "";  // raise a flag, throw an exception, ...
}
wilhelmtell
  • 57,473
  • 20
  • 96
  • 131
2

You're supposed to use the ISO C++ conformant version _getcwd I think. There's no point returning a const string, and you should use free to deallocate (at least according to MSDN):

string getcwd()
{
    char* a_cwd = _getcwd(NULL, 0);
    string s_cwd(a_cwd);
    free(a_cwd);
    return s_cwd;
}

Of course you should also check if _getcwd() returns NULL.

Alex Korban
  • 14,916
  • 5
  • 44
  • 55
1

You need to check for a_cwd being NULL. Then it will work on Mac, Windows, Linux. However, it's not POSIX-compliant.

EDIT: perror doesn't exit the program, so you should exit, throw an exception, or do something.

Matthew Flaschen
  • 278,309
  • 50
  • 514
  • 539
  • Got it, copy-pasting from MSDN :) And I think the POSIX standard is stupid here, as there is no way to securely allocate a char* that is guaranteed to be large enough >:s – rubenvb May 19 '10 at 21:48
  • @ruben, I don't agree. _PC_PATH_MAX is the recommended method. It's important to note that getcwd will never buffer overflow, since you pass it the size. At most it will give you a ERANGE if your buffer is too small. MSDN is not a reliable source on POSIX. – Matthew Flaschen May 19 '10 at 21:52
  • Never said anything about this needing to be POSIX. I'm dropping full POSIX compatibility here, and gladly :) – rubenvb May 19 '10 at 22:04
  • @ruben, you said "the POSIX standard is stupid here". What I pointed out is that if it only needed to work on POSIX, there's a recommended solution. Naturally working cross-platform adds complexity. – Matthew Flaschen May 19 '10 at 22:07
  • I understand, and am regretting my stubborn decision for "no boost or Qt" to keep my program nice and small. Thanks though. – rubenvb May 19 '10 at 22:10
1

How about this? It's short, exception safe, and doesn't leak.

std::string getcwd() {
    std::string result(1024,'\0');
    while( getcwd(&result[0], result.size()) == 0) {
        if( errno != ERANGE ) {
          throw std::runtime_error(strerror(errno));
        }
        result.resize(result.size()*2);
    }   
    result.resize(result.find('\0'));
    return result;
}
JoeG
  • 12,994
  • 1
  • 38
  • 63
  • A little more and I'd give you a +1 but unfortunately you have that -1 in `result.resize(result.find('\0')-1);` and thus your result is wrong... – Alexis Wilke Oct 27 '12 at 03:09
0

When "string constructor" do everything for you:

#include <stdio.h>  // defines FILENAME_MAX
#include <unistd.h> // for getcwd()

std::string GetCurrentWorkingDir()
{
    std::string cwd("\0",FILENAME_MAX+1);
    return getcwd(&cwd[0],cwd.capacity());
}
IluxaKuk
  • 389
  • 3
  • 7
  • Doesn't seem like a safe thing to do: "Unlike PATH_MAX, this macro is defined even if there is no actual limit imposed. In such a case, its value is typically a very large number. This is always the case on GNU/Hurd systems." I don't want to allocate a possible large amount of memory for a trivially small result. – rubenvb Apr 26 '19 at 13:44
  • Thanks for pointing this out.You are absolutely correct about large number, I used this approach in my program because I didn't have such restrictions. This solution works in Linux/Windows also. – IluxaKuk Apr 26 '19 at 14:51
  • 1
    @lluxaKuk well, thing is that this constant may change in future versions of glibc and Visual Studio, so it seems like a dangerous thing to rely on. – rubenvb Apr 26 '19 at 14:58