Why does the BackupWrite function take a pointer to a modifiable buffer when it shouldn’t be modifying the buffer?


The Backup­Write function takes a non-const pointer to the buffer to be written to the file being restored. Will it actually modify the buffer? Assuming it doesn't, why wasn't it declared const? It would be much more convenient if it took a const pointer to the buffer, so that people with const buffers didn't have to const_cast every time they called the function. Would changing the parameter from non-const to const create any compatibility problems?

Okay, let's take the questions in order.

Will it actually modify the buffer? No.

Why wasn't it declared const? My colleague Aaron Margosis explained that the function dates back to Windows NT 3.1, when const-correctness was rarely considered. A lot of functions from that area (particularly in the kernel) suffer from the same problem. For example, the computer name passed to the Reg­Connect­Registry function is a non-const pointer even though the function never writes to it.

Last question: Can the parameter be changed from non-const to const without breaking compatibility?

It would not cause problems from a binary compatibility standpoint, because a const pointer and a non-const pointer take the same physical form in Win32. However, it breaks source code compatiblity. Consider the following code fragment:

BOOL WINAPI TestModeBackupWrite(
  HANDLE hFile,
  LPBYTE lpBuffer,
  DWORD nNumberOfBytesToWrite,
  LPDWORD lpNumberOfBytesWritten,
  BOOL bAbort,
  BOOL bProcessSecurity,
  LPVOID *lpContext)
{
 ... simulate a BackupWrite ...
 return TRUE;
}

BOOL (WINAPI *BACKUPWRITEPROC)(HANDLE, LPBYTE, DWORD,
                 LPDWORD, BOOL, BOOL, LPVOID *);
BACKUPWRITEPROC TestableBackupWrite;

void SetTestMode(bool testing)
{
 if (testing) {
  TestableBackupWrite = TestModeBackupWrite;
 } else {
  TestableBackupWrite = BackupWrite;
 }
}

The idea here is that the program can be run in test mode, say to do a simulated restore. (You see this sort of thing a lot with DVD-burning software.) The program uses Testable­Backup­Write whenever it wants to write to a file being restored from backup. In test mode, Testable­Backup­Write points to the Test­Mode­Backup­Write function; in normal mode, it points to the Backup­Write function.

If the second parameter were changed from LPBYTE to const BYTE *, then the above code would hit a compiler error.

Mind you, maybe it's worth breaking some source code in order to get better const-correctness, but for now, the cost/benefit tradeoff biases toward leaving things alone.

Comments (18)
  1. anonymouscommenter says:

    Speaking of const BYTE*, it's funny that the core headers do not declare LPCBYTE, but that you have to include winscard.h to get it.

  2. Ken Hagan says:

    The cost of not changing the prototype is that anyone who has genuinely read-only data that they wish to add to the backup is forced to allocate a buffer and make a writeable copy to feed to the function. Merely using a cast isn't sufficient because the API doesn't promise not to write to the buffer and we're good programmers who don't exploit undocumented behaviour (however plausible that behaviour might be).

    In contrast, the cost of changing the prototype (at least in the example here) *is* simply a cast.

    My guess is that the number of people who would benefit from a change in prototype far exceeds the number who would be inconvenienced and that the benefit greatly exceeds the inconvenience.

  3. anonymouscommenter says:

    The answer to Ken Hagan's complaint is simple. Document the buffer is never written to.

  4. anonymouscommenter says:

    Well the SAL annotation on the parameter says it's an 'in', so I guess it has that going for it in addition to the documentation. Kind of like the annotation on the command line parameter for CreateProcess says in/out, which is not immediately obvious and could perhaps lead one to assume things incorrectly if it were not there. I'm guessing Microsoft would fix any incorrect SAL annotations that they would find or be made aware of.

  5. VinDuv says:

    I’m wondering why the TestableBackupWrite = BackupWrite; assignment is not allowed (after the proposed change to BackupWrite).

    The modified BackupWrite is allowed to modify fewer of its arguments than TestableBackupWrite; calls to TestableBackupWrite can be replaced with calls to BackupWrite without introducing compilation issues (since a X* can be silently converted to a const X*). So shouldn’t the compiler allow the assignment? Is it just an arbitrary limitation of the C type checking system, or am I missing something?

  6. anonymouscommenter says:

    @VinDuv: Even if that limitation did not exist, people could write stuff like:

    auto TestableBackupWrite = BackupWrite;

    if (testing) TestableBackupWrite = TestModeBackupWrite;

    and that would try to assign the non-const parameter version to a const parameter type.

  7. anonymouscommenter says:

    It's been a while since I've done any C, but I think the example code is missing the `typedef` keyword when declaring the function pointer type.  Not that it matters much, as I can't see many people trying to use this sample.

  8. anonymouscommenter says:

    @VinDuv: Known bug in C language specification.

  9. anonymouscommenter says:

    "…but for now, the cost/benefit tradeoff biases toward leaving things alone"

    The chief problem with that line of thinking, and I see it all the time, is that the cost/benefit analysis always ignores the fact that the longer you wait to change something, the harder it becomes.

    Better to just be honest and say "…but the cost/benefit tradeoff will never allow this to be changed."

  10. Alexander Riccio says:

    @meh CreateProcess arguments *do* actually need to be writable, as Raymond's written about before. And no, Microsoft is (with lots of respect!) pretty bad about fixing broken SAL in headers.

    Speaking of SAL, there's a specific annotation for this sort of backwards-compatibility problem: `_Const_`

  11. Timothy Byrd (ETAP) says:

    I've been in C++ land too long.

    Why not add an override that takes a const pointer?

    (And have one method call the other.)

  12. Timothy Byrd (ETAP) says:

    I've been in C++ land too long.

    Why not add an *overload* that takes a const pointer?

    (And have one method call the other.)

  13. anonymouscommenter says:

    Because no matter how hard Microsoft tries to ditch it, C isn't gone. The function is extern "C" as library functions must be, so it would have to be inline… which could still cause problems with function pointers.

    A worse case where const-correctness is concerned is DrawText and its flag to modify the text, despite taking a const pointer. So does ShellAboutW, but this one isn't supposed to be caĺled by user code anyway.

  14. anonymouscommenter says:

    @Alexander Riccio: As Raymond did indeed write, only CreateProcessW needs to have a writeable input.  The ANSI version CreateProcessA can take a read-only input since it constructs a temporary buffer for converting the string to Unicode, which is then writeable.

    blogs.msdn.com/…/9673254.aspx

  15. anonymouscommenter says:

    Seems like this is more of a "const/benefit analysis"…

  16. anonymouscommenter says:

    For CreateProcessW, the right migration to const correctness would be: Deprecate the current call, and introduce a new one which have separate parameters for the executable path name and for the rest of the command line. This way, there would be no need for CreateProcessW to guess where the executable path name ends (when " is not used as the first character).

    Not only could the new parameters be flagged as "const", also this terrible guessing would be eliminated.

  17. Burov Dmitry says:

    i think you can have both function declarations in the header-files, making it look like an overloaded by parameters function ?

    Somewhat similr to how you have both declarations for CreateProcessW and for CreateProcess while it is not two functions and a single one with an optional shortcut for it.

  18. anonymouscommenter says:

    @CreateProcessW: That's an awful lot of work for very little benefit, and CreateProcess already takes separate arguments for the executable path and the command line, although using both is optional for callers; perhaps you mean to change the semantics to make them both required?  Remember, all features start at -100 points.

    @Burov Dmitry: Adding an overload with a const (obviously in C++ only) will help in many situations, but it can cause code which previously compiled correctly to now give compiler errors.  For example, if you tried to take the address of CreateProcess in certain contexts, or if you tried to use it as a template parameter, the overload resolution might fail.

Comments are closed.

Skip to main content