1

I would like to copy all files from test1 into test2. The code compiles but nothing happens.

#include <iostream>
#include <stdlib.h>
#include <windows.h>

using namespace std;

int main()
{
    string input1 = "C:\\test1\\";
    string input2 = "C:\\test2\\";
    MoveFile(input1.c_str(), input2.c_str());
}

I was considering xcopy but it would not accept a pre defined string. Is there a work around?

Donald Duck
  • 8,409
  • 22
  • 75
  • 99
Robbie J
  • 79
  • 1
  • 9
  • 3
    Check the return value of `MoveFile` and when you see it says that it failed, use `GetLastError` to find out why. – Scott Hunter Mar 01 '17 at 15:21
  • 2
    Per the [`MoveFile()`](https://msdn.microsoft.com/en-us/library/windows/desktop/aa365239.aspx) documentation: "*`lpNewFileName` [in] The new name for the file or directory. **The new name must not already exist**. A new file may be on a different file system or drive. A new directory must be on the same drive.*" Does the `test2` directory already exist? Consider using [`SHFileOperation()`](https://msdn.microsoft.com/en-us/library/windows/desktop/bb762164.aspx) or [`IFileOperation`](https://msdn.microsoft.com/en-us/library/windows/desktop/bb775771.aspx) instead of `MoveFile()`. – Remy Lebeau Mar 01 '17 at 17:09
  • 1
    If these are both directories then what you hope to happen won't. – David Heffernan Mar 01 '17 at 19:19

2 Answers2

2
std::string GetLastErrorAsString()
{
    //Get the error message, if any.
    DWORD errorMessageID = ::GetLastError();
    if (errorMessageID == 0)
        return std::string(); //No error message has been recorded

    LPSTR messageBuffer = nullptr;
    size_t size = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
        NULL, errorMessageID, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR)&messageBuffer, 0, NULL);

    std::string message(messageBuffer, size);

    //Free the buffer.
    LocalFree(messageBuffer);

    return message;
}
int main()
{
    string input1 = "C:\\test1\\";
    string input2 = "C:\\test2\\";
    if (!MoveFile(input1.c_str(), input2.c_str()))
    {
        string msg = GetLastErrorAsString(); 
        cout << "fail: " << msg << endl;
    }
    else {
        cout << "ok" << endl;
    }
    system("pause");
}

Your code works for me, you may have to set the character set to use multi-byte character set in your project properties. If not, provide us with the error. Check if you have the write rights on C:. Check if there already is a test2 folder in C: (or if there is not a test1 folder in C:).

user
  • 934
  • 6
  • 17
  • `GetLastErrorAsString()` leaks memory in case the `std::string` c'tor raises an exception. It also calls `GetLastError()` tenuously late. A better implementation has been published in [this documentation topic](http://stackoverflow.com/documentation/winapi/2573/error-reporting-and-handling/9378/). – IInspectable Mar 01 '17 at 16:38
  • Also, writing to `std::cout` may indirectly cause `GetLastError()` to reset. It is meaningless to call `GetLastError()` if `MoveFile()` succeeds. You must call `GetLastError()` only if `MoveFile()` fails, and must do so before any other Win32 API functions are called, eg: `if (!MoveFile(input1.c_str(), input2.c_str())) { string msg = GetLastErrorAsString(); cout << "fail: " << msg << endl; } else { cout << "ok" << endl; }` – Remy Lebeau Mar 01 '17 at 17:11
  • @RemyLebeau Thank you for pointing this out, I edited the answer with it – user Mar 01 '17 at 17:51
  • @IInspectable Good point, I've looked at your link and will definitely update this answer (and my own GetLastError function) with it (but very likely this will be tomorrow) – user Mar 01 '17 at 17:53
  • I would have used a simple `std::vector` or even a fixed-size array instead of `std::unique_ptr` with a custom deleter. You don't have to let `FormatMessage()` allocate the buffer memory. But if you do, you can [use `LocalFree()` directly as the deleter](http://stackoverflow.com/a/15067510/65863) (even on Windows 10, despite the warning in the documentation). You don't need to use a lambda to delegate to `HeapFree()`. – Remy Lebeau Mar 01 '17 at 18:50
1

I resovled the issue by removing the \\ from test2. Folder test 2 doesn't exist. Thank you for the replies and the test code. I think SHFileOperation will be a better option as I have to transfer files from a floppy to my C drive. string input1 = "C:\\test1\\"; string input2 = "C:\\test2";

Robbie J
  • 79
  • 1
  • 9