Re: Patch : gdbserver get_image_name on CE

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 14 June 2009 09:47:05, Danny Backx wrote:
> > IIRC, psapi.dll does exist on WinCE, but it is
> > not bundled with the OS usually.  If I'm not confusing
> > it with some other dll, I think I have it for ARM, gotten
> > from some MSFT redistributable, and it does work.
> 
> It isn't on the Windows Embedded for x86 that I'm using. But the
> gdbserver code appears to be well written : it silently falls back to
> something that does work.
> 
> I've not messed with that code because "if it ain't broke, don't fix
> it". I could have surrounded it with #ifndef _WIN32_WCE but I didn't see
> the point.

I've no idea what you're talking about here, or even why
we talking about psapi.dll anyway.  The only hunks in your
patches touching psapi related things are:


-  if (!load_psapi ())
+  if (!load_psapi ()) {
     goto failed;
+  }

and...

+  /* Note : no psapi.dll on CE, fall back to get_image_name below. */
   if (!psapi_get_dll_name ((DWORD) event->lpBaseOfDll, dll_buf))


The fact that psapi is dynamicaly loaded with LoadLibrary
was exactly so that gdbserver falls back to other mechanisms.
Even on desktop Windows it isn't garanteed that psapi.dll will be
around.  Please drop these spurious hunks from the patch.

BTW, 

-  if (!load_psapi ())
+  if (!load_psapi ()) {
     goto failed;
+  }

The coding conventions states to not surround a single statement
with curly braces, so that original version would be the correct
form --- your patch would make it wrong.


+  /* Note : no psapi.dll on CE, fall back to get_image_name below. */

Double spaces after period, no space before ':'.  This would be:

  /* Note: no psapi.dll on CE, fall back to get_image_name below.  */

But as I said, this hunk is unnecessary.

-- 
Pedro Alves