warning C6273: Non-integer passed as _Param_(2) when an integer is required

Question

I have this method:

void CMeetingScheduleAssistantDlg::RestartProgram()
{
    RESTART_THIS_MFC_APP;
}

It calls a macro as defined:

#define RESTART_THIS_MFC_APP \
    { \
        HWND _hWndMain = AfxGetApp()->m_pMainWnd->GetSafeHwnd(); \
        DWORD _dwProcessID = (DWORD)_getpid(); \
        TCHAR _cFilespec[_MAX_PATH + 1]; \
        if (::GetModuleFileName(AfxGetInstanceHandle(), _cFilespec,_MAX_PATH) == 0) \
            AfxMessageBox(_T("ERROR: Could not obtain full filespec of this application to restart it."), MB_OK|MB_ICONSTOP); \
        else \
        { \
            CString strAppFilespec(_cFilespec); \
; \
            LPTSTR lpszFilename = ::PathFindFileName(_cFilespec); \
            _tcscpy_s(lpszFilename,_MAX_PATH - (lpszFilename - _cFilespec),_T("AppRestarter.exe") ); \
; \
            CString strAppRestarterFilespec(_cFilespec); \
; \
            CString strCmdLine ; \
            strCmdLine.Format(_T("%ld %ld \"%s\""),_hWndMain,_dwProcessID,(LPCTSTR)strAppFilespec); \
; \
            HINSTANCE hProcess = ::ShellExecute(NULL,_T("open"),(LPCTSTR)strAppRestarterFilespec, \
                                                (LPCTSTR)strCmdLine,_T(".\\"), SW_HIDE); \
            if ( (DWORD)(DWORD_PTR)hProcess <= 32) \
                AfxMessageBox(_T("ERROR: Could not run application restarter."), MB_OK|MB_ICONSTOP); \
        } \
    }

The above is a third party macro that was provided to me that works in conjunction with a application restarter executable. The original author is no longer available.

The application builds and it functions correctly. But I am getting a code analysis error:

warning C6273: Non-integer passed as Param(2) when an integer is required in call to 'ATL::CStringT >

::Format' Actual type: 'struct HWND__ *': if a pointer value is being passed, %p should be used.

It is true that _hWndMain is of type HWND but I can't use %p as the format code. If I do that then the application restarter fails and tells me it expects this parameter to be numeric.

So as you can see I reverted to using %ld which I thought would be right.

Can this warning be suppressed by a change to my code? Or is this just a false positive I must live with?

score 2 · Accepted Answer · edited May 23 '17 at 12:34

It is not a false positive; the warning is correct and caught a bug. %ld is not right because pointers are not integers. The language specification is clear on this fact, so %ld is wrong because it instructs the function to interpret the pointer value as a long integer. One of the many ways this can go wrong is when targeting a 64-bit processor, where pointers are 64 bits wide but integers (including longs, are only 32 bits wide). %ld will chop off the upper bits.

With CString<T>::Format, you must play by the same rules as printf, which is why the warning is telling you that you need to use %p to pass pointer values. The reason why an HWND is considered a pointer is because that's the way the type is defined in the Windows headers. It's not technically a pointer, just an opaque value, but the pointer semantics are generally appropriate and avoid bugs.

You say that using %p is not an option because you get an error that the parameter is not numeric. I'm guessing that's because %p is causing the value to be printed with some kind of format that CString<T>::Format considers appropriate for a pointer value, perhaps something in hexadecimal, maybe even with a 0x prefix, but the function you're passing that formatted string to is not expecting that.

Thus, you need to explicitly interpret the HWND value as a numeric value before using it with %ld. This will not only silence the warning, but give you the correct semantics. But you need to pick carefully which type you'll use to represent it. Your first instinct might be size_t, but this is not guaranteed by the standard to have the same width as a pointer. It will "work" on Windows 32-bit and 64-bit, but is not technically valid. Instead, to play it safe, prefer intptr_t or uintptr_t, depending on whether you want a signed or unsigned representation of the address. For the format string, use either %Id, %Ix, or %IX. From the looks of your current code, you probably want %Id. You may also need to explicitly indicate the size of the field, depending on what format is expected by the consumer.

strCmdLine.Format(_T("%Id %ld \"%s\""),
                  reinterpret_cast<intptr_t>(_hWndMain),
                  _dwProcessID,
                  static_cast<LPCTSTR>(strAppFilespec));

You'll need to make sure that you've included <inttypes.h>.

(Note that the %I format specifier is a Microsoft invention and therefore not portable. On other compilers, you would use the standard macros from inttypes.h. I don't think these exist in Microsoft's implementation, although I don't have a compiler in front of me. Perhaps you can prove me wrong?)

Thank you for a very thorough explanation and worked example. I am trying this out right now. :) Can I kindly ask why you have chosen to use `reinterpret_cast` and `static_cast`? — Andrew Truckle, Dec 24 '16 at 14:05
For [the standard reasons](http://stackoverflow.com/questions/103512). You're writing in C++, so you should prefer [the C++ style casts](http://stackoverflow.com/questions/28002). You need `reinterpret_cast` to convert a pointer to an integer, since pointers are not integers. This is the very definition of a "reinterpretation". However, to get a string pointer out of a `CString` object you do not want a reinterpretation—you just want to call the cast operator defined by the class. Avoiding the C-style cast ensures the right thing happens, and the code is self-documenting. @andrew — Cody Gray - on strike, Dec 24 '16 at 14:09
Interesting! Thanks. I have over 150 places in my application where it just uses a `(LPCTSTR)` as the cast for a `CString` or a `COleDateTime::Format` return value. That will take a while to change ... :) — Andrew Truckle, Dec 24 '16 at 14:16
@andrew The C-style cast will still work, of course. You don't have to go refactoring a major code base. That's more likely to introduce bugs than fix them! But if I were you, I would certainly write all *new* code this way. — Cody Gray - on strike, Dec 24 '16 at 14:42
You don't really need the `static_cast`. The `CString` class template provides the [CString::GetString](https://msdn.microsoft.com/en-us/library/sddk80xf.aspx#csimplestringt__getstring) member function, that returns a C-style string. — IInspectable, Dec 24 '16 at 15:35

warning C6273: Non-integer passed as _Param_(2) when an integer is required

1 Answers1