3
\$\begingroup\$

I've made a small click-to-copy clipboard util in WinAPI that doesn't steal focus, stays on top of all windows, can be dragged and simulates Ctrl+V keystrokes if needed.

Clipboard util

What can be improved and are there any pitfalls I am overseeing? I am aware of checking if the call to Clipboard succeeded and that I should avoid this one:

(WPARAM)GetStockObject(DEFAULT_GUI_FONT)

Here is the source code:

// A tool to quickly copy the strings to clipboard
#pragma comment(linker,"\"/manifestdependency:type='win32' \
name='Microsoft.Windows.Common-Controls' version='6.0.0.0' \
processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
#include <windows.h>
#include "resource.h"
#include <string>

static wchar_t szWindowClass[] = L"myclass2.0";
static wchar_t szTitle[] = L"Ron's utils";
HINSTANCE hInst;
LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);

//=========================== global constants ===========================
// positions and dimensions
const int formydelta = 95;
const int formwidth = 775;
const int formheight = 55;
const int buttony = 15;
const int buttonx = 15;
const int buttonxdelta = 85;
BOOL pastedirectly = false;

// hard coded strings
const std::string sotexthelp = "Please take [the tour](https://stackoverflow.com/tour) and read [the help page](https://stackoverflow.com/help). Welcome to SO.";
const std::string sotextbooks = "Sounds like you could benefit from reading one of [these C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list).";
//========================================================================

// copy to clipboard function
void copytoclipboard(const std::string& s){
    HGLOBAL hMem = GlobalAlloc(GMEM_MOVEABLE, s.size() + 1);
    memcpy((LPWSTR)GlobalLock(hMem), s.c_str(), s.size() + 1);
    GlobalUnlock(hMem);
    OpenClipboard(0);
    EmptyClipboard();
    if (!SetClipboardData(CF_TEXT, hMem)){
        GlobalFree(hMem);
    }
    CloseClipboard();
}
// paste from clipboard function:
void pastefromclipboard(){
    if (pastedirectly) {
        INPUT myinput[4];
        SecureZeroMemory(myinput, sizeof(myinput));
        myinput[0].type = INPUT_KEYBOARD;
        myinput[0].ki.wVk = VK_LCONTROL;
        myinput[1].type = INPUT_KEYBOARD;
        myinput[1].ki.wVk = 'V';
        myinput[2].type = INPUT_KEYBOARD;
        myinput[2].ki.wVk = 'V';
        myinput[2].ki.dwFlags = KEYEVENTF_KEYUP;
        myinput[3].type = INPUT_KEYBOARD;
        myinput[3].ki.wVk = VK_LCONTROL;
        myinput[3].ki.dwFlags = KEYEVENTF_KEYUP;
        SendInput(4, myinput, sizeof(INPUT));
    }
}
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow){
    WNDCLASSEX wcex;
    wcex.cbSize = sizeof(WNDCLASSEX);
    wcex.style = CS_HREDRAW | CS_VREDRAW | CS_DROPSHADOW;
    wcex.lpfnWndProc = WndProc;
    wcex.cbClsExtra = 0;
    wcex.cbWndExtra = 0;
    wcex.hInstance = hInstance;
    wcex.hIcon = LoadIcon(hInstance, MAKEINTRESOURCE(IDI_ICON1));
    wcex.hCursor = LoadCursor(NULL, IDC_ARROW);
    wcex.hbrBackground = (HBRUSH)(COLOR_WINDOW);
    wcex.lpszMenuName = NULL;
    wcex.lpszClassName = szWindowClass;
    wcex.hIconSm = LoadIcon(wcex.hInstance, MAKEINTRESOURCE(IDI_ICON1));

    if (!RegisterClassEx(&wcex)){
        MessageBox(NULL, L"Call to RegisterClassEx failed!", L"cppexamples.com", NULL);
        return 1;
    }

    hInst = hInstance;
    HWND hWnd = CreateWindowEx(
        WS_EX_NOACTIVATE | WS_EX_TOOLWINDOW | WS_EX_TOPMOST,
        szWindowClass,
        szTitle,
        WS_POPUP | WS_BORDER | WS_VISIBLE,
        ((GetSystemMetrics(SM_CXSCREEN) - formwidth) / 2), (GetSystemMetrics(SM_CYSCREEN) - formydelta),
        formwidth, // width
        formheight, // height
        NULL,
        NULL,
        hInstance,
        NULL
        );

    if (!hWnd){
        MessageBox(NULL, L"Call to CreateWindow failed!", L"cppexamples.com", NULL);
        return 1;
    }
    SetWindowPos(hWnd, HWND_TOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
    ShowWindow(hWnd, nCmdShow);
    UpdateWindow(hWnd);

    MSG msg;
    while (GetMessage(&msg, NULL, 0, 0)){
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }
    return (int)msg.wParam;
}

LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam){
    PAINTSTRUCT ps;
    HDC hdc;

    switch (message){
    case WM_CREATE:
    {
        // button 1
        HWND hwndButton = CreateWindow(
            L"BUTTON",
            L"Email",
            WS_VISIBLE | WS_CHILD,
            buttonx, // x position 
            buttony, // y position 
            75, // width
            24, // height
            hWnd,
            (HMENU)101, // ID
            hInst,
            NULL);
        SendMessage(hwndButton, WM_SETFONT, (WPARAM)GetStockObject(DEFAULT_GUI_FONT), 0);

        // code for other buttons omitted
        // button exit
        HWND hwndButton110 = CreateWindow(
            L"BUTTON",
            L"Exit",
            WS_VISIBLE | WS_CHILD,
            formwidth - 90,
            buttony,
            75,
            24,
            hWnd,
            (HMENU)110,
            hInst,
            NULL);
        SendMessage(hwndButton110, WM_SETFONT, (WPARAM)GetStockObject(DEFAULT_GUI_FONT), 0);
        //checkbox
        HWND hwndCheckBox = CreateWindowEx(0, L"BUTTON", L"Paste directly",
            BS_AUTOCHECKBOX | WS_CHILD | WS_VISIBLE,
            formwidth - 180, buttony + 3,
            90, 20,
            hWnd,
            (HMENU)111,
            hInst,
            0);
        SendMessage(hwndCheckBox, WM_SETFONT, (WPARAM)GetStockObject(DEFAULT_GUI_FONT), 0);
    }
    break;
    case WM_COMMAND: // process commands
        switch (LOWORD(wParam)) {
        case 101:
        {
            copytoclipboard("sample string 1");
            pastefromclipboard();
        }
        break;
        // code for other buttons omitted
        case 110:
        {
            OpenClipboard(0);
            EmptyClipboard();
            PostQuitMessage(0);
        }
        break;
        case 111: // checkbox
        {
            pastedirectly = IsDlgButtonChecked(hWnd, 111);
        }
        break;
        }
        break;
    case WM_NCHITTEST: {
        LRESULT hit = DefWindowProc(hWnd, message, wParam, lParam);
        if (hit == HTCLIENT) hit = HTCAPTION;
        return hit;
    }
    break;
    case WM_PAINT:
        hdc = BeginPaint(hWnd, &ps);

        EndPaint(hWnd, &ps);
        break;
    case WM_DESTROY:
        PostQuitMessage(0);
        break;
    default:
        return DefWindowProc(hWnd, message, wParam, lParam);
        break;
    }
    return 0;
}
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

This code hardly has any error handling. You never check return values of essential functions, such as GlobalLock, OpenClipboard and never call GetLastError to figure out what caused failure.

There are magic numbers scattered everywhere from UI dimensions to command ids.

Fancy problem: this code is not exception-safe. If string constructor at call to copytoclipboard fails (out of memory for example) the exception will raise out of WndProc callback causing undefined behavior since calling C code is not aware of C++ exceptions. If your application supposed to handle low memory scenarios then you can call copytoclipboard inside of try..catch block and manually deal with thrown exceptions. Or (as alternative) you can mark WndProc and other callbacks with noexcept specifier so compiler won't let sudden exceptions to get into C code and will call terminate.

\$\endgroup\$
5
  • \$\begingroup\$ Appreciate the feedback. I will implement the error checking / handling. How should I ago about replacing the C++ std::string? \$\endgroup\$
    – Raw N
    Commented Sep 30, 2017 at 20:59
  • \$\begingroup\$ Maybe use the LPCWSTR pointer instead? How likely is it the std::string ctor will raise an exception? \$\endgroup\$
    – Raw N
    Commented Sep 30, 2017 at 21:06
  • \$\begingroup\$ @Ron If your application supposed to handle low memory scenarios then you can call copytoclipboard inside of try..catch block and manually deal with thrown exceptions. Or (as alternative) you can mark WndProc and other callbacks with noexcept specifier so compiler won't let sudden exceptions to get into C code and will call terminate. Using LPCWSTR would be just dropping handy C++ class. It could work when you are passing come constant though. \$\endgroup\$ Commented Sep 30, 2017 at 21:11
  • \$\begingroup\$ Awesome info. Would appreciate if you could incorporate the comment into an answer. \$\endgroup\$
    – Raw N
    Commented Sep 30, 2017 at 21:13
  • \$\begingroup\$ Also worth mentioning this is for my personal needs, not production. \$\endgroup\$
    – Raw N
    Commented Sep 30, 2017 at 21:20
0
\$\begingroup\$

I will be speaking as I am not the author and I need to maintain this program.

  • There is no alternative to direct pasting implemented; it only emulates Ctrl+V right now and the program won't work with checkbox not enabled.
  • The identificators of UI elements are hard to keep an eye on. The better way of dealing with it is having enum with human readable labels to which you assign the integers if you need to. Handles could be named better as well.
  • And of course there are so many better ways of writing the GUI other than hard-coding all coordinates inside the code. Visual Studio is definitely the way to go if you want to write something maintainable in WinAPI GUI.
\$\endgroup\$
1
  • \$\begingroup\$ @Ron well this is code review and in this specific case it only matters for someone who maintains the program. If you take a second look years after writing something you will be puzzled if the code is not self-documenting. \$\endgroup\$ Commented Sep 25, 2017 at 15:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.