MZ_Exec() patch, try 3

Alright, yet another revision of this patch.  This one uses dynamic memory allocation via HeapAlloc/Free(), doesn't modify the input command line and doesn't try to close process handles if CreateProcess failed.  Third time a charm? ;-)

* dlls/winedos/module.c:
Chris Morgan <cmorgan(a)alum.wpi.edu>
Call CreateProcessA() when executing non-dos applications from a dos application.




Index: dlls/winedos/module.c
===================================================================
RCS file: /home/wine/wine/dlls/winedos/module.c,v
retrieving revision 1.23
diff -u -r1.23 module.c
--- dlls/winedos/module.c	18 Oct 2002 23:48:58 -0000	1.23
+++ dlls/winedos/module.c	21 Oct 2002 22:00:16 -0000
@@ -339,17 +339,78 @@
 
 /***********************************************************************
  *		MZ_Exec
+ *
+ * this may only be called from existing DOS processes
  */
 BOOL WINAPI MZ_Exec( CONTEXT86 *context, LPCSTR filename, BYTE func, LPVOID paramblk )
 {
-  /* this may only be called from existing DOS processes
-   * (i.e. one DOS app spawning another) */
-  /* FIXME: do we want to check binary type first, to check
-   * whether it's a NE/PE executable? */
+  DWORD binType;
+  STARTUPINFOA st;
+  PROCESS_INFORMATION pe;
+  BOOL status;
+
   HANDLE hFile = CreateFileA( filename, GENERIC_READ, FILE_SHARE_READ,
 			     NULL, OPEN_EXISTING, 0, 0);
   BOOL ret = FALSE;
+
+  if(!GetBinaryTypeA(filename, &binType))   /* determine what kind of binary this is */
+  {
+    return FALSE; /* binary is not an executable */
+  }
+
   if (hFile == INVALID_HANDLE_VALUE) return FALSE;
+
+  /* handle non-dos executables */
+  if(binType != SCS_DOS_BINARY)
+  {
+    if(func == 0) /* load and execute */
+    {
+      LPBYTE fullCmdLine;
+      WORD fullCmdLength;
+      LPBYTE psp_start = (LPBYTE)((DWORD)DOSVM_psp << 4);
+      PDB16 *psp = (PDB16 *)psp_start;
+      ExecBlock *blk = (ExecBlock *)paramblk;
+      LPBYTE cmdline = PTR_REAL_TO_LIN(SELECTOROF(blk->cmdline),OFFSETOF(blk->cmdline));
+      LPBYTE envblock = PTR_REAL_TO_LIN(psp->environment, 0);
+      BYTE cmdLength = cmdline[0];
+
+      fullCmdLength = (strlen(filename) + 1) + cmdLength + 1; /* filename + space + cmdline + terminating null character */
+
+      fullCmdLine = HeapAlloc(GetProcessHeap(), 0, fullCmdLength);
+
+      /* build the full command line from the executable file and the command line being passed in */
+      snprintf(fullCmdLine, fullCmdLength, "%s ", filename); /* start off with the executable filename and a space */
+      memcpy(fullCmdLine + strlen(fullCmdLine), cmdline + 1, cmdLength); /* append cmdline onto the end */
+      fullCmdLine[fullCmdLength - 1] = 0; /* null terminate string */
+
+      ZeroMemory (&st, sizeof(STARTUPINFOA));
+      st.cb = sizeof(STARTUPINFOA);
+      status = CreateProcessA (NULL, fullCmdLine, NULL, NULL, TRUE,
+               0, envblock, NULL, &st, &pe);
+
+      WaitForSingleObject(pe.hProcess, INFINITE);  /* wait here until the child process is complete */
+
+      HeapFree(GetProcessHeap(), 0, fullCmdLine);  /* free the memory we allocated */
+
+      /* clean up PROCESS_INFORMATION handles if necessary */
+      if(status)
+      {
+        CloseHandle(pe.hProcess);
+        CloseHandle(pe.hThread);
+      }
+
+      ret = TRUE;
+    } else
+    {
+      FIXME("EXEC type of %d not implemented for non-dos executables\n", func);
+      ret = FALSE;
+    }
+
+    CloseHandle(hFile);
+    return ret;
+  } /* if(binType != SCS_DOS_BINARY) */
+
+  /* handle dos executables */
   switch (func) {
   case 0: /* load and execute */
   case 1: /* load but don't execute */

[Dmitry Timoshkov]

<chrismorgan(a)rcn.com> wrote:
> HANDLE hFile = CreateFileA( filename, GENERIC_READ, FILE_SHARE_READ,
>        NULL, OPEN_EXISTING, 0, 0);
>    BOOL ret = FALSE;
> +
> +  if(!GetBinaryTypeA(filename, &binType))   /* determine what kind of binary this is */
> +  {
> +    return FALSE; /* binary is not an executable */
> +  }

You are leaking the file handle here. Better move the GetBinaryTypeA call
before the CreateFileA.
> +      status = CreateProcessA (NULL, fullCmdLine, NULL, NULL, TRUE,
> +               0, envblock, NULL, &st, &pe);
> +
> +      WaitForSingleObject(pe.hProcess, INFINITE);  /* wait here until the child process is complete */
> +
> +      HeapFree(GetProcessHeap(), 0, fullCmdLine);  /* free the memory we allocated */
> +
> +      /* clean up PROCESS_INFORMATION handles if necessary */
> +      if(status)
> +      {
> +        CloseHandle(pe.hProcess);
> +        CloseHandle(pe.hThread);
> +      }

Potential wait on an invalid pe.hProcess. Better:

if (satus)
{
    WaitForSingleObject(pe.hProcess, INFINITE);
    CloseHandle(pe.hProcess);
    CloseHandle(pe.hThread);
}

-- 
Dmitry.

[Chris Morgan]

Stop leaking a hFile, don't WaitForSingleObject() if CreateProcess() fails, 
error out if memory allocation fails.

* dlls/winedos/module.c:
Chris Morgan <cmorgan(a)alum.wpi.edu>
Call CreateProcessA() when executing non-dos applications from a dos 
application.

[Dmitry Timoshkov]

"Chris Morgan" <cmorgan(a)alum.wpi.edu> wrote:
> + hFile = CreateFileA( filename, GENERIC_READ, FILE_SHARE_READ,
> + NULL, OPEN_EXISTING, 0, 0);
> +
> if (hFile == INVALID_HANDLE_VALUE) return FALSE;
> ...
> + fullCmdLine = HeapAlloc(GetProcessHeap(), 0, fullCmdLength);
> + if(!fullCmdLine) return FALSE; /* return false on memory alloc failure */

You are still leaking a file handle here.

Why remove a CreateFileA call out of the reach of your new code since
you don't use hFile at all? 

-- 
Dmitry.

[Chris Morgan]

Damnit all.  One more time.  Thanks for the help Dimitry ;-)

Chris



On Monday 21 October 2002 07:52 pm, Dmitry Timoshkov wrote:
> "Chris Morgan" <cmorgan(a)alum.wpi.edu> wrote:
> > + hFile = CreateFileA( filename, GENERIC_READ, FILE_SHARE_READ,
> > + NULL, OPEN_EXISTING, 0, 0);
> > +
> > if (hFile == INVALID_HANDLE_VALUE) return FALSE;
>
> ...
> > + fullCmdLine = HeapAlloc(GetProcessHeap(), 0, fullCmdLength);
> > + if(!fullCmdLine) return FALSE; /* return false on memory alloc failure
> > */
>
> You are still leaking a file handle here.
>
> Why remove a CreateFileA call out of the reach of your new code since
> you don't use hFile at all?