[Lldb-commits] [PATCH] D70742: [LLDB] [Windows] Avoid using InitializeContext for allocating a CONTEXT. NFC.

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

I'm good with the change, but have a couple small requests.  I hope to hear from others, too, as this area is outside my wheelhouse.



================
Comment at: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:158
+  memset(&m_context, 0, sizeof(m_context));
+  m_context.ContextFlags = kWinContextFlags;
   if (::SuspendThread(
----------------
As far as I understand what InitializeContext does, this seems a suitable replacement for how it's used here.

But if someone were to change the flag to include CONTEXT_XSTATE, then this would no longer work, because the xstate is variable length.

I would suggest:

1.  Eliminate the kWinContextFlags (defined at the top of this file) and just use CONTEXT_ALL here.  The extra name doesn't seem to clarify anything, and the distance between the definition and usage can make it harder for folks to understand this code.

2.  Add a comment saying this is a substitute for InitializeContext for Wine, which makes searching the code for `InitializeContext` useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70742/new/

https://reviews.llvm.org/D70742