Zeroes buffer for DocumentProperti

printing/backend/win_helper.cc

Index: printing/backend/win_helper.cc
diff --git a/printing/backend/win_helper.cc b/printing/backend/win_helper.cc
index bb6b69e7424ad5646c7e82c2506ab5e76497b3d3..30db31ec1a317694f7f8bb2605cd073346aa5409 100644
--- a/printing/backend/win_helper.cc
+++ b/printing/backend/win_helper.cc
@@ -478,13 +478,16 @@ scoped_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   scoped_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(malloc(buffer_size)));
+  memset(out.get(), 0, buffer_size);

[Lei Zhang, 2014/10/08 20:11:34]

Just calloc() instead?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:18:15]

Done.

   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER;
   if (DocumentProperties(
           NULL, printer, const_cast<wchar_t*>(L""), out.get(), in, flags) !=
       IDOK) {
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   }
-  CHECK_GE(buffer_size, out.get()->dmSize + out.get()->dmDriverExtra);
+  WORD size = out->dmSize;
+  WORD extra_size = out->dmDriverExtra;
+  CHECK_GE(buffer_size, size + extra_size);

[Lei Zhang, 2014/10/08 20:11:34]

Do we care if size + extra_size overflows?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:18:15]

Done.

[Lei Zhang, 2014/10/08 20:24:15]

I just checked and a WORD is only 16-bit. There's no way 2 WORDS added together
can overflow a LONG. So why not just: LONG actual_size = out->dmSize +
out->dmDriverExtra; ?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:32:55]

C++ does WORD+WORD->WORD, so old version indeed had overflow problem.
However it made no difference because overflow would likely satisfy CHECK_GE

LONG would work instead of int, but we don't need LONG for api calls like above,
so int should be used, according our style.

Also I put two local variable hoping to see them in future crash dumps on stack.


On 2014/10/08 20:24:15, Lei Zhang wrote:

   return out.Pass();
 }
 
@@ -505,6 +508,7 @@ scoped_ptr<DEVMODE, base::FreeDeleter> PromptDevMode(
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   scoped_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(malloc(buffer_size)));
+  memset(out.get(), 0, buffer_size);
   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER | DM_IN_PROMPT;
   LONG result = DocumentProperties(window,
                                    printer,
@@ -516,7 +520,9 @@ scoped_ptr<DEVMODE, base::FreeDeleter> PromptDevMode(
     *canceled = (result == IDCANCEL);
   if (result != IDOK)
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
-  CHECK_GE(buffer_size, out.get()->dmSize + out.get()->dmDriverExtra);
+  WORD size = out->dmSize;
+  WORD extra_size = out->dmDriverExtra;
+  CHECK_GE(buffer_size, size + extra_size);
   return out.Pass();
 }