You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2001/09/09 08:03:05 UTC

cvs commit: apr/threadproc/win32 proc.c signals.c

wrowe       01/09/08 23:03:05

  Modified:    .        CHANGES
               file_io/netware filesys.c
               file_io/os2 filesys.c
               file_io/unix filepath.c
               file_io/win32 filepath.c filesys.c
               include  apr_file_info.h apr_thread_proc.h
               include/arch/os2 fileio.h
               include/arch/win32 fileio.h threadproc.h
               test     testmmap.c
               threadproc/win32 proc.c signals.c
  Log:
    Fix the apr_proc_create for win32.  In order to do so, this patch
    introduces a flags arg for apr_filepath_get(), like apr_filepath_merge(),
    that allows the APR_FILEPATH_NATIVE result format.
  
    This launches win32 processes with the Unicode semantics (although it
    runs sbcs apps equally well) and changes the default to 'not detached',
    in sync with the unix default.
  
    Finally, assures apr_filepath_get() uses the '/' semantics on OS2 by
    default.
  
  Revision  Changes    Path
  1.152     +12 -4     apr/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/apr/CHANGES,v
  retrieving revision 1.151
  retrieving revision 1.152
  diff -u -r1.151 -r1.152
  --- CHANGES	2001/09/08 23:36:34	1.151
  +++ CHANGES	2001/09/09 06:03:04	1.152
  @@ -1,10 +1,18 @@
   Changes with APR b1  
   
  -  *) Add the new thread read/write lock API to APR.
  -     [Aaron Bannert <aa...@clove.org>]
  +  *) Fix the API for the apr_proc_create() call on Win32.  Several
  +     bad assumptions are gone, including a mismatch between unix and
  +     win32, where win32 was defaulting to create detached.  Also fixes
  +     the apr_proc_t's pid member to a real pid (identity that works
  +     across processes) instead of the handle (which is a new hproc
  +     member value.)  [William Rowe]
   
  -  *) Add the new thread mutex lock API to APR.
  -     [Aaron Bannert <aa...@clove.org>]
  +  *) Modify the external apr_filepath_get() fn to take a flags arg,
  +     currently only for APR_FILEPATH_NATIVE.  This returns c:\foo
  +     format on Win32, and should do the same on OS2, or sys\vol:\foo
  +     on Netware.  Primarily for internals, but possibly useful to
  +     others (and it mirrors some of the other apr_filepath_*() calls.)
  +     [William Rowe]
   
     *) Cache GMT offset on platforms that don't store it in the tm struct.
        This offset is normalized to be independent of daylight savings
  
  
  
  1.2       +1 -1      apr/file_io/netware/filesys.c
  
  Index: filesys.c
  ===================================================================
  RCS file: /home/cvs/apr/file_io/netware/filesys.c,v
  retrieving revision 1.1
  retrieving revision 1.2
  diff -u -r1.1 -r1.2
  --- filesys.c	2001/08/29 23:32:07	1.1
  +++ filesys.c	2001/09/09 06:03:05	1.2
  @@ -102,7 +102,7 @@
       return -1;
   }
   
  -APR_DECLARE(apr_status_t) apr_filepath_get(char **rootpath,
  +APR_DECLARE(apr_status_t) apr_filepath_get(char **rootpath, apr_int32_t flags,
                                              apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
  
  
  
  1.3       +18 -8     apr/file_io/os2/filesys.c
  
  Index: filesys.c
  ===================================================================
  RCS file: /home/cvs/apr/file_io/os2/filesys.c,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- filesys.c	2001/09/02 05:10:41	1.2
  +++ filesys.c	2001/09/09 06:03:05	1.3
  @@ -100,7 +100,8 @@
   }
   
   
  -apr_status_t filepath_drive_get(char **rootpath, char drive, apr_pool_t *p)
  +apr_status_t filepath_drive_get(char **rootpath, char drive, 
  +                                apr_int32_t flags, apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
       char *pos;
  @@ -117,12 +118,11 @@
           return APR_FROM_OS_ERROR(rc);
       }
   
  -    /* ###: We really should consider adding a flag to allow the user
  -     * to have the APR_FILEPATH_NATIVE result
  -     */
  -    for (pos=path; *pos; pos++) {
  -        if (*pos == '\\')
  -            *pos = '/';
  +    if (!(flags & APR_FILEPATH_NATIVE)) {
  +        for (pos=path; *pos; pos++) {
  +            if (*pos == '\\')
  +                *pos = '/';
  +        }
       }
   
       *rootpath = apr_pstrdup(p, path);
  @@ -142,12 +142,14 @@
   }
   
   
  -APR_DECLARE(apr_status_t) apr_filepath_get(char **defpath, apr_pool_t *p)
  +APR_DECLARE(apr_status_t) apr_filepath_get(char **defpath, apr_int32_t flags,
  +                                           apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
       ULONG drive;
       ULONG drivemap;
       ULONG rv, pathlen = sizeof(path) - 3;
  +    char *pos;
   
       DosQueryCurrentDisk(&drive, &drivemap);
       path[0] = '@' + drive;
  @@ -155,6 +157,14 @@
       rv = DosQueryCurrentDir(drive, path+3, &pathlen);
   
       *defpath = apr_pstrdup(p, path);
  +
  +    if (!(flags & APR_FILEPATH_NATIVE)) {
  +        for (pos=*defpath; *pos; pos++) {
  +            if (*pos == '\\')
  +                *pos = '/';
  +        }
  +    }
  +
       return APR_SUCCESS;
   }    
   
  
  
  
  1.8       +3 -2      apr/file_io/unix/filepath.c
  
  Index: filepath.c
  ===================================================================
  RCS file: /home/cvs/apr/file_io/unix/filepath.c,v
  retrieving revision 1.7
  retrieving revision 1.8
  diff -u -r1.7 -r1.8
  --- filepath.c	2001/08/24 01:53:45	1.7
  +++ filepath.c	2001/09/09 06:03:05	1.8
  @@ -71,7 +71,8 @@
   
   /* Any OS that requires/refuses trailing slashes should be dealt with here.
    */
  -APR_DECLARE(apr_status_t) apr_filepath_get(char **defpath, apr_pool_t *p)
  +APR_DECLARE(apr_status_t) apr_filepath_get(char **defpath, apr_int32_t flags,
  +                                           apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
       if (!getcwd(path, sizeof(path))) {
  @@ -170,7 +171,7 @@
            * passing the address of a char const* for a char** arg.
            */
           char *getpath;
  -        rv = apr_filepath_get(&getpath, p);
  +        rv = apr_filepath_get(&getpath, apr_int32_t flags, p);
           rootpath = getpath;
           if (rv != APR_SUCCESS)
               return errno;
  
  
  
  1.13      +2 -2      apr/file_io/win32/filepath.c
  
  Index: filepath.c
  ===================================================================
  RCS file: /home/cvs/apr/file_io/win32/filepath.c,v
  retrieving revision 1.12
  retrieving revision 1.13
  diff -u -r1.12 -r1.13
  --- filepath.c	2001/09/01 05:11:46	1.12
  +++ filepath.c	2001/09/09 06:03:05	1.13
  @@ -418,10 +418,10 @@
           char *getpath;
   #ifndef NETWARE
           if (addtype == APR_EINCOMPLETE && addroot[1] == ':')
  -            rv = filepath_drive_get(&getpath, addroot[0], p);
  +            rv = filepath_drive_get(&getpath, addroot[0], flags, p);
           else
   #endif
  -            rv = apr_filepath_get(&getpath, p);
  +            rv = apr_filepath_get(&getpath, flags, p);
           if (rv != APR_SUCCESS)
               return rv;
           basepath = getpath;
  
  
  
  1.3       +13 -16    apr/file_io/win32/filesys.c
  
  Index: filesys.c
  ===================================================================
  RCS file: /home/cvs/apr/file_io/win32/filesys.c,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- filesys.c	2001/09/02 05:10:41	1.2
  +++ filesys.c	2001/09/09 06:03:05	1.3
  @@ -116,7 +116,8 @@
   }
   
   
  -apr_status_t filepath_drive_get(char **rootpath, char drive, apr_pool_t *p)
  +apr_status_t filepath_drive_get(char **rootpath, char drive, 
  +                                apr_int32_t flags, apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
   #if APR_HAS_UNICODE_FS
  @@ -149,12 +150,11 @@
           if (!GetFullPathName(drivestr, sizeof(path), path, &ignored))
               return apr_get_os_error();
       }
  -    /* ###: We really should consider adding a flag to allow the user
  -     * to have the APR_FILEPATH_NATIVE result
  -     */
  -    for (*rootpath = path; **rootpath; ++*rootpath) {
  -        if (**rootpath == '\\')
  -            **rootpath = '/';
  +    if (!(flags & APR_FILEPATH_NATIVE)) {
  +        for (*rootpath = path; **rootpath; ++*rootpath) {
  +            if (**rootpath == '\\')
  +                **rootpath = '/';
  +        }
       }
       *rootpath = apr_pstrdup(p, path);
       return APR_SUCCESS;
  @@ -201,7 +201,7 @@
   }
   
   
  -APR_DECLARE(apr_status_t) apr_filepath_get(char **rootpath,
  +APR_DECLARE(apr_status_t) apr_filepath_get(char **rootpath, apr_int32_t flags,
                                              apr_pool_t *p)
   {
       char path[APR_PATH_MAX];
  @@ -222,12 +222,11 @@
           if (!GetCurrentDirectory(sizeof(path), path))
               return apr_get_os_error();
       }
  -    /* ###: We really should consider adding a flag to allow the user
  -     * to have the APR_FILEPATH_NATIVE result
  -     */
  -    for (*rootpath = path; **rootpath; ++*rootpath) {
  -        if (**rootpath == '\\')
  -            **rootpath = '/';
  +    if (!(flags & APR_FILEPATH_NATIVE)) {
  +        for (*rootpath = path; **rootpath; ++*rootpath) {
  +            if (**rootpath == '\\')
  +                **rootpath = '/';
  +        }
       }
       *rootpath = apr_pstrdup(p, path);
       return APR_SUCCESS;
  @@ -257,5 +256,3 @@
       }
       return APR_SUCCESS;
   }
  -
  -
  
  
  
  1.24      +5 -2      apr/include/apr_file_info.h
  
  Index: apr_file_info.h
  ===================================================================
  RCS file: /home/cvs/apr/include/apr_file_info.h,v
  retrieving revision 1.23
  retrieving revision 1.24
  diff -u -r1.23 -r1.24
  --- apr_file_info.h	2001/08/18 15:15:46	1.23
  +++ apr_file_info.h	2001/09/09 06:03:05	1.24
  @@ -357,10 +357,13 @@
    * Return the default file path (for relative file names)
    * @ingroup apr_filepath
    * @param path the default path string returned
  + * @param flags optional flag APR_FILEPATH_NATIVE to retrieve the
  + *              default file path in os-native format.
    * @param p the pool to allocate the default path string from
  - * @deffunc apr_status_t apr_filepath_get(char **path, apr_pool_t *p)
  + * @deffunc apr_status_t apr_filepath_get(char **path, apr_int32_t flags, apr_pool_t *p)
    */
  -APR_DECLARE(apr_status_t) apr_filepath_get(char **path, apr_pool_t *p);
  +APR_DECLARE(apr_status_t) apr_filepath_get(char **path, apr_int32_t flags,
  +                                           apr_pool_t *p);
   
   /**
    * Set the default file path (for relative file names)
  
  
  
  1.74      +6 -0      apr/include/apr_thread_proc.h
  
  Index: apr_thread_proc.h
  ===================================================================
  RCS file: /home/cvs/apr/include/apr_thread_proc.h,v
  retrieving revision 1.73
  retrieving revision 1.74
  diff -u -r1.73 -r1.74
  --- apr_thread_proc.h	2001/08/27 03:17:15	1.73
  +++ apr_thread_proc.h	2001/09/09 06:03:05	1.74
  @@ -124,6 +124,12 @@
       apr_file_t *out;
       /** Parent's side of pipe to child's stdouterr */
       apr_file_t *err;
  +#ifdef WIN32
  +    /** Must retain the handle as any clone may not have the
  +     *  the same permissions 
  +     */
  +    HANDLE hproc;
  +#endif
   };
   
   typedef struct apr_thread_t           apr_thread_t;
  
  
  
  1.27      +2 -1      apr/include/arch/os2/fileio.h
  
  Index: fileio.h
  ===================================================================
  RCS file: /home/cvs/apr/include/arch/os2/fileio.h,v
  retrieving revision 1.26
  retrieving revision 1.27
  diff -u -r1.26 -r1.27
  --- fileio.h	2001/09/01 05:06:26	1.26
  +++ fileio.h	2001/09/09 06:03:05	1.27
  @@ -104,7 +104,8 @@
   #define IS_FNCHAR(c) c_is_fnchar[(unsigned char)c]
   
   apr_status_t filepath_root_test(char *path, apr_pool_t *p);
  -apr_status_t filepath_drive_get(char **rootpath, char drive, apr_pool_t *p);
  +apr_status_t filepath_drive_get(char **rootpath, char drive, 
  +                                apr_int32_t flags, apr_pool_t *p);
   apr_status_t filepath_root_case(char **rootpath, char *root, apr_pool_t *p);
   
   #endif  /* ! FILE_IO_H */
  
  
  
  1.56      +19 -1     apr/include/arch/win32/fileio.h
  
  Index: fileio.h
  ===================================================================
  RCS file: /home/cvs/apr/include/arch/win32/fileio.h,v
  retrieving revision 1.55
  retrieving revision 1.56
  diff -u -r1.55 -r1.56
  --- fileio.h	2001/08/28 21:45:04	1.55
  +++ fileio.h	2001/09/09 06:03:05	1.56
  @@ -91,11 +91,25 @@
   
   typedef apr_uint16_t apr_wchar_t;
   
  +/* Helper functions for the WinNT ApiW() functions.  APR treats all
  + * resource identifiers (files, etc) by their UTF-8 name, to provide 
  + * access to all named identifiers.  [UTF-8 completely maps Unicode 
  + * into char type strings.]
  + *
  + * The _path flavors below provide us fast mappings of the
  + * Unicode filename //?/D:/path and //?/UNC/mach/share/path mappings,
  + * which allow unlimited (well, 32000 wide character) length names.
  + * These prefixes may appear in Unicode, but must not appear in the
  + * Ascii API calls.  So we tack them on in utf8_to_unicode_path, and
  + * strip them right back off in unicode_to_utf8_path.
  + */
   apr_status_t utf8_to_unicode_path(apr_wchar_t* dststr, apr_size_t dstchars, 
                                     const char* srcstr);
   apr_status_t unicode_to_utf8_path(char* dststr, apr_size_t dstchars, 
                                     const apr_wchar_t* srcstr);
   
  +/* XXX: ONE OF THESE IS WRONG, WHO CHANGED IT?
  + */
   #define APR_FILE_MAX MAX_PATH
   #else /* !APR_HAS_UNICODE_FS */
   #define APR_FILE_MAX MAX_PATH
  @@ -233,8 +247,12 @@
    *
    * But we need to figure out what the cwd of a given volume is,
    * when the user passes D:foo.  This fn will determine D:'s cwd.
  + *
  + * If flags includes the bit APR_FILEPATH_NATIVE, the path returned
  + * is in the os-native format.
    */
  -apr_status_t filepath_drive_get(char **rootpath, char drive, apr_pool_t *p);
  +apr_status_t filepath_drive_get(char **rootpath, char drive, 
  +                                apr_int32_t flags, apr_pool_t *p);
   
   
   /* If the user passes d: vs. D: (or //mach/share vs. //MACH/SHARE),
  
  
  
  1.16      +0 -1      apr/include/arch/win32/threadproc.h
  
  Index: threadproc.h
  ===================================================================
  RCS file: /home/cvs/apr/include/arch/win32/threadproc.h,v
  retrieving revision 1.15
  retrieving revision 1.16
  diff -u -r1.15 -r1.16
  --- threadproc.h	2001/09/01 05:10:23	1.15
  +++ threadproc.h	2001/09/09 06:03:05	1.16
  @@ -82,7 +82,6 @@
   
   struct apr_procattr_t {
       apr_pool_t *cntxt;
  -    STARTUPINFO si;
       apr_file_t *parent_in;
       apr_file_t *child_in;
       apr_file_t *parent_out;
  
  
  
  1.30      +1 -1      apr/test/testmmap.c
  
  Index: testmmap.c
  ===================================================================
  RCS file: /home/cvs/apr/test/testmmap.c,v
  retrieving revision 1.29
  retrieving revision 1.30
  diff -u -r1.29 -r1.30
  --- testmmap.c	2001/08/01 21:06:26	1.29
  +++ testmmap.c	2001/09/09 06:03:05	1.30
  @@ -99,7 +99,7 @@
       }
       fprintf(stdout,"OK\n");
       
  -    apr_filepath_get(&file1, context);
  +    apr_filepath_get(&file1, 0, context);
       file1 = apr_pstrcat(context,file1,"/testmmap.c",NULL);
   
       fprintf(stdout, "Opening file........................");
  
  
  
  1.48      +191 -152  apr/threadproc/win32/proc.c
  
  Index: proc.c
  ===================================================================
  RCS file: /home/cvs/apr/threadproc/win32/proc.c,v
  retrieving revision 1.47
  retrieving revision 1.48
  diff -u -r1.47 -r1.48
  --- proc.c	2001/07/13 13:37:24	1.47
  +++ proc.c	2001/09/09 06:03:05	1.48
  @@ -74,24 +74,9 @@
   APR_DECLARE(apr_status_t) apr_procattr_create(apr_procattr_t **new,
                                                     apr_pool_t *cont)
   {
  -    (*new) = (apr_procattr_t *)apr_palloc(cont, sizeof(apr_procattr_t));
  -
  -    if ((*new) == NULL) {
  -        return APR_ENOMEM;
  -    }
  +    (*new) = (apr_procattr_t *)apr_pcalloc(cont, sizeof(apr_procattr_t));
       (*new)->cntxt = cont;
  -    (*new)->parent_in = NULL;
  -    (*new)->child_in = NULL;
  -    (*new)->parent_out = NULL;
  -    (*new)->child_out = NULL;
  -    (*new)->parent_err = NULL;
  -    (*new)->child_err = NULL;
  -    (*new)->currdir = NULL; 
       (*new)->cmdtype = APR_PROGRAM;
  -    (*new)->detached = TRUE;
  -
  -    memset(&(*new)->si, 0, sizeof((*new)->si));
  -
       return APR_SUCCESS;
   }
   
  @@ -127,7 +112,7 @@
   
   static apr_status_t make_handle_private(apr_file_t *file)
   {
  -    HANDLE pid = GetCurrentProcess();
  +    HANDLE hproc = GetCurrentProcess();
       HANDLE filehand = file->filehand;
   
       /* Create new non-inheritable versions of handles that
  @@ -135,8 +120,8 @@
        * inherits these handles; resulting in non-closeable handles
        * to the respective pipes.
        */
  -    if (!DuplicateHandle(pid, filehand,
  -                         pid, &file->filehand, 0,
  +    if (!DuplicateHandle(hproc, filehand,
  +                         hproc, &file->filehand, 0,
                            FALSE, DUPLICATE_SAME_ACCESS))
           return apr_get_os_error();
       /* 
  @@ -160,9 +145,9 @@
           return apr_get_os_error();
       else
       {
  -        HANDLE pid = GetCurrentProcess();
  -        if (!DuplicateHandle(pid, original->filehand, 
  -                             pid, &duplicate->filehand, 0,
  +        HANDLE hproc = GetCurrentProcess();
  +        if (!DuplicateHandle(hproc, original->filehand, 
  +                             hproc, &duplicate->filehand, 0,
                                TRUE, DUPLICATE_SAME_ACCESS))
               return apr_get_os_error();
       }
  @@ -276,25 +261,11 @@
   APR_DECLARE(apr_status_t) apr_procattr_dir_set(apr_procattr_t *attr,
                                                 const char *dir) 
   {
  -    char path[MAX_PATH];
  -    int length;
  -
  -    if (dir[0] != '\\' && dir[0] != '/' && dir[1] != ':') { 
  -        length = GetCurrentDirectory(MAX_PATH, path);
  -
  -        if (length == 0 || length + strlen(dir) + 1 >= MAX_PATH)
  -            return APR_ENOMEM;
  -
  -        attr->currdir = apr_pstrcat(attr->cntxt, path, "\\", dir, NULL);
  -    }
  -    else {
  -        attr->currdir = apr_pstrdup(attr->cntxt, dir);
  -    }
  -
  -    if (attr->currdir) {
  -        return APR_SUCCESS;
  -    }
  -    return APR_ENOMEM;
  +    /* curr dir must be in native format, there are all sorts of bugs in
  +     * the NT library loading code that flunk the '/' parsing test.
  +     */
  +    return apr_filepath_merge(&attr->currdir, NULL, dir, 
  +                              APR_FILEPATH_NATIVE, attr->cntxt);
   }
   
   APR_DECLARE(apr_status_t) apr_procattr_cmdtype_set(apr_procattr_t *attr,
  @@ -311,11 +282,6 @@
       return APR_SUCCESS;
   }
   
  -/* TODO:  
  - *   apr_proc_create with APR_SHELLCMD on Win9x won't work due to MS KB:
  - *   Q150956
  - */
  -
   APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
                                             const char *progname,
                                             const char * const *args,
  @@ -323,19 +289,20 @@
                                             apr_procattr_t *attr,
                                             apr_pool_t *cont)
   {
  -    apr_size_t i, iEnvBlockLen;
  +    apr_status_t rv;
  +    apr_oslevel_e os_level;
  +    apr_size_t i;
       char *cmdline;
  -    char ppid[20];
  -    char *envstr;
  -    char *pEnvBlock, *pNext;
  +    char *pEnvBlock;
       PROCESS_INFORMATION pi;
       DWORD dwCreationFlags = 0;
   
       new->in = attr->parent_in;
       new->err = attr->parent_err;
       new->out = attr->parent_out;
  +
  +    (void) apr_get_oslevel(cont, &os_level);
   
  -    attr->si.cb = sizeof(attr->si);
       if (attr->detached) {
           /* If we are creating ourselves detached, Then we should hide the
            * window we are starting in.  And we had better redfine our
  @@ -344,68 +311,56 @@
            * not manage the stdio handles properly when running old 16
            * bit executables if the detached attribute is set.
            */
  -        apr_oslevel_e os_level;
  -        if (apr_get_oslevel(cont, &os_level) == APR_SUCCESS) {
  -            if (os_level >= APR_WIN_NT) {
  -                dwCreationFlags |= DETACHED_PROCESS;
  -            }
  +        if (os_level >= APR_WIN_NT) {
  +            /* 
  +             * XXX DETACHED_PROCESS won't on Win9x at all; on NT/W2K 
  +             * 16 bit executables fail (MS KB: Q150956)
  +             */
  +            dwCreationFlags |= DETACHED_PROCESS;
           }
  -        attr->si.dwFlags = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES;
  -        attr->si.wShowWindow = SW_HIDE;
  -
  -        if (attr->child_in) {
  -            attr->si.hStdInput = attr->child_in->filehand;
  -        }
  -
  -        if (attr->child_out) {
  -            attr->si.hStdOutput = attr->child_out->filehand;
  -        }
  -
  -        if (attr->child_err) {
  -            attr->si.hStdError = attr->child_err->filehand;
  -        }
       }
   
  -    if (attr->cmdtype == APR_PROGRAM) {
  -        const char *ptr = progname;
  -
  -        if (*ptr =='"') {
  -            ptr++;
  -        }
  -
  -        if (*ptr == '\\' || *ptr == '/' || *++ptr == ':') {
  -            cmdline = apr_pstrdup(cont, progname);
  -        }
  -        else if (attr->currdir == NULL) {
  -            cmdline = apr_pstrdup(cont, progname);
  -        }
  -        else {
  -            char lastchar = attr->currdir[strlen(attr->currdir)-1];
  -            if ( lastchar == '\\' || lastchar == '/') {
  -                cmdline = apr_pstrcat(cont, attr->currdir, progname, NULL);
  -            }
  -            else {
  -                cmdline = apr_pstrcat(cont, attr->currdir, "\\", progname, NULL);
  -            }
  -        }
  -    }
  -    else {
  -        char * shell_cmd = getenv("COMSPEC");
  -        if (!shell_cmd)
  -            shell_cmd = SHELL_PATH;
  -        shell_cmd = apr_pstrdup(cont, shell_cmd);
  -        cmdline = apr_pstrcat(cont, shell_cmd, " /C ", progname, NULL);
  +    /* progname must be the unquoted, actual name, or NULL if this is
  +     * a 16 bit app running in the VDM or WOW context.
  +     */
  +    if (progname[0] == '\"') {
  +        progname = apr_pstrndup(cont, progname + 1, strlen(progname) - 2);
       }
  +    
  +    /* progname must be unquoted, in native format, as there are all sorts 
  +     * of bugs in the NT library loader code that fault when parsing '/'.
  +     * We do not directly manipulate cmdline, and it will become a copy,
  +     * so we've casted past the constness issue.
  +     */
  +    if (strchr(progname, ' '))
  +        cmdline = apr_pstrcat(cont, "\"", progname, "\"", NULL);
  +    else
  +        cmdline = (char*)progname;
   
       i = 1;
       while (args && args[i]) {
  -        cmdline = apr_pstrcat(cont, cmdline, " ", args[i], NULL);
  +        if (strchr(args[i], ' '))
  +            cmdline = apr_pstrcat(cont, cmdline, " \"", args[i], "\"", NULL);
  +        else
  +            cmdline = apr_pstrcat(cont, cmdline, " ", args[i], NULL);
           i++;
       }
   
  -    _itoa(_getpid(), ppid, 10);
  -    if (env) {
  -        envstr = apr_pstrcat(cont, "parentpid=", ppid, NULL);
  +    if (attr->cmdtype == APR_SHELLCMD) {
  +        char *shellcmd = getenv("COMSPEC");
  +        if (!shellcmd)
  +            shellcmd = SHELL_PATH;
  +        if (shellcmd[0] == '"')
  +            progname = apr_pstrndup(cont, shellcmd + 1, strlen(shellcmd) - 1);
  +        else if (strchr(shellcmd, ' '))
  +            shellcmd = apr_pstrcat(cont, "\"", shellcmd, "\"", NULL);
  +        cmdline = apr_pstrcat(cont, shellcmd, " /C \"", cmdline, "\"", NULL);
  +    }
  +
  +    if (!env) 
  +        pEnvBlock = NULL;
  +    else {
  +        apr_size_t iEnvBlockLen;
           /*
            * Win32's CreateProcess call requires that the environment
            * be passed in an environment block, a null terminated block of
  @@ -417,58 +372,139 @@
               iEnvBlockLen += strlen(env[i]) + 1;
               i++;
           }
  -  
  -        pEnvBlock = (char *)apr_pcalloc(cont, iEnvBlockLen + strlen(envstr));
  +        if (!i) 
  +            ++iEnvBlockLen;
  +
  +#if APR_HAS_UNICODE_FS
  +        if (os_level >= APR_WIN_NT) {
  +            apr_wchar_t *pNext;
  +            pEnvBlock = (char *)apr_palloc(cont, iEnvBlockLen * 2);
  +            dwCreationFlags |= CREATE_UNICODE_ENVIRONMENT;
  +
  +            i = 0;
  +            pNext = (apr_wchar_t*)pEnvBlock;
  +            while (env[i]) {
  +                apr_size_t in = strlen(env[i]) + 1;
  +                if ((rv = conv_utf8_to_ucs2(env[i], &in, 
  +                                            pNext, &iEnvBlockLen)) 
  +                        != APR_SUCCESS) {
  +                    return rv;
  +                }
  +                pNext = wcschr(pNext, L'\0') + 1;
  +                i++;
  +            }
  +	    if (!i)
  +                *(pNext++) = L'\0';
  +	    *pNext = L'\0';
  +        }
  +        else 
  +#endif /* APR_HAS_UNICODE_FS */
  +        {
  +            char *pNext;
  +            pEnvBlock = (char *)apr_palloc(cont, iEnvBlockLen);
       
  -        i = 0;
  -        pNext = pEnvBlock;
  -        while (env[i]) {
  -            strcpy(pNext, env[i]);
  -            pNext = pNext + strlen(pNext) + 1;
  -            i++;
  +            i = 0;
  +            pNext = pEnvBlock;
  +            while (env[i]) {
  +                strcpy(pNext, env[i]);
  +                pNext = strchr(pNext, '\0') + 1;
  +                i++;
  +            }
  +	    if (!i)
  +                *(pNext++) = '\0';
  +	    *pNext = '\0';
           }
  -        strcpy(pNext, envstr); 
  -	pNext = pNext + strlen(pNext) + 1;
  -	*pNext = '\0';
  +    } 
  +
  +#if APR_HAS_UNICODE_FS
  +    if (os_level >= APR_WIN_NT)
  +    {
  +        STARTUPINFOW si;
  +        apr_size_t nprg = strlen(progname) + 1;
  +        apr_size_t ncmd = strlen(cmdline) + 1, nwcmd = ncmd;
  +        apr_size_t ncwd = strlen(attr->currdir) + 1, nwcwd = ncwd;
  +        apr_wchar_t *wprg = apr_palloc(cont, nprg * 2);
  +        apr_wchar_t *wcmd = apr_palloc(cont, ncmd * 2);
  +        apr_wchar_t *wcwd = apr_palloc(cont, ncwd * 2);
  +
  +        if (((rv = utf8_to_unicode_path(wprg, &nprg, progname)) 
  +                    != APR_SUCCESS)
  +         || ((rv = conv_utf8_to_ucs2(cmdline, &ncmd, wcmd, &nwcmd)) 
  +                    != APR_SUCCESS)
  +         || ((rv = conv_utf8_to_ucs2(attr->currdir, &ncwd, wcwd, &nwcwd)) 
  +                    != APR_SUCCESS)) {
  +            return rv;
  +        }
  +
  +        memset(&si, 0, sizeof(si));
  +        si.cb = sizeof(si);
  +        if (attr->detached) {
  +            si.dwFlags |= STARTF_USESHOWWINDOW;
  +            si.wShowWindow = SW_HIDE;
  +        }
  +        if (attr->child_in->filehand || attr->child_out->filehand
  +                                     || attr->child_err->filehand) {
  +            si.dwFlags |= STARTF_USESTDHANDLES;
  +            si.hStdInput = attr->child_in->filehand;
  +            si.hStdOutput = attr->child_out->filehand;
  +            si.hStdError = attr->child_err->filehand;
  +        }
  +        rv = CreateProcessW(wprg, wcmd,        /* Command line */
  +                            NULL, NULL,        /* Proc & thread security attributes */
  +                            TRUE,              /* Inherit handles */
  +                            dwCreationFlags,   /* Creation flags */
  +                            pEnvBlock,         /* Environment block */
  +                            wcwd,     /* Current directory name */
  +                            &si, &pi);
       }
       else {
  -        SetEnvironmentVariable("parentpid", ppid);
  -        pEnvBlock = NULL;
  -    } 
  -    
  +#endif /* APR_HAS_UNICODE_FS */
  +        STARTUPINFOA si;
  +        memset(&si, 0, sizeof(si));
  +        si.cb = sizeof(si);
  +        if (attr->detached) {
  +            si.dwFlags |= STARTF_USESHOWWINDOW;
  +            si.wShowWindow = SW_HIDE;
  +        }
  +        if (attr->child_in->filehand || attr->child_out->filehand
  +                                     || attr->child_err->filehand) {
  +            si.dwFlags |= STARTF_USESTDHANDLES;
  +            si.hStdInput = attr->child_in->filehand;
  +            si.hStdOutput = attr->child_out->filehand;
  +            si.hStdError = attr->child_err->filehand;
  +        }
  +
  +        rv = CreateProcessA(progname, cmdline, /* Command line */
  +                            NULL, NULL,        /* Proc & thread security attributes */
  +                            TRUE,              /* Inherit handles */
  +                            dwCreationFlags,   /* Creation flags */
  +                            pEnvBlock,         /* Environment block */
  +                            attr->currdir,     /* Current directory name */
  +                            &si, &pi);
  +    }
   
  -    if (CreateProcess(NULL, cmdline,     /* Command line */
  -                      NULL, NULL,        /* Proc & thread security attributes */
  -                      TRUE,              /* Inherit handles */
  -                      dwCreationFlags,   /* Creation flags */
  -                      pEnvBlock,         /* Environment block */
  -                      attr->currdir,     /* Current directory name */
  -                      &attr->si, &pi)) {
  -
  -        // TODO: THIS IS BADNESS
  -        // The completion of the apr_proc_t type leaves us ill equiped to track both 
  -        // the pid (Process ID) and handle to the process, which are entirely
  -        // different things and each useful in their own rights.
  -        //
  -        // Signals are broken since the hProcess varies from process to process, 
  -        // while the true process ID would not.
  -        new->pid = (pid_t) pi.hProcess;
  -
  -        if (attr->child_in) {
  -            apr_file_close(attr->child_in);
  -        }
  -        if (attr->child_out) {
  -            apr_file_close(attr->child_out);
  -        }
  -        if (attr->child_err) {
  -            apr_file_close(attr->child_err);
  -        }
  -        CloseHandle(pi.hThread);
  +    /* Check CreateProcess result 
  +     */
  +    if (!rv)
  +        return apr_get_os_error();
   
  -        return APR_SUCCESS;
  +    /* XXX Orphaned handle warning - no fix due to broken apr_proc_t api.
  +     */
  +    new->hproc = pi.hProcess;
  +    new->pid = pi.dwProcessId;
  +
  +    if (attr->child_in) {
  +        apr_file_close(attr->child_in);
  +    }
  +    if (attr->child_out) {
  +        apr_file_close(attr->child_out);
       }
  +    if (attr->child_err) {
  +        apr_file_close(attr->child_err);
  +    }
  +    CloseHandle(pi.hThread);
   
  -    return apr_get_os_error();
  +    return APR_SUCCESS;
   }
   
   APR_DECLARE(apr_status_t) apr_proc_wait(apr_proc_t *proc, apr_wait_how_e wait)
  @@ -477,21 +513,24 @@
       if (!proc)
           return APR_ENOPROC;
       if (wait == APR_WAIT) {
  -        if ((stat = WaitForSingleObject((HANDLE)proc->pid, 
  +        if ((stat = WaitForSingleObject(proc->hproc, 
                                           INFINITE)) == WAIT_OBJECT_0) {
  +            CloseHandle(proc->hproc);
  +            proc->hproc = NULL;
               return APR_CHILD_DONE;
           }
           else if (stat == WAIT_TIMEOUT) {
               return APR_CHILD_NOTDONE;
           }
  -        return GetLastError();
  +        return apr_get_os_error();
       }
  -    if ((stat = WaitForSingleObject((HANDLE)proc->pid, 0)) == WAIT_OBJECT_0) {
  +    if ((stat = WaitForSingleObject((HANDLE)proc->hproc, 0)) == WAIT_OBJECT_0) {
  +        CloseHandle(proc->hproc);
  +        proc->hproc = NULL;
           return APR_CHILD_DONE;
       }
       else if (stat == WAIT_TIMEOUT) {
           return APR_CHILD_NOTDONE;
       }
  -    return GetLastError();
  +    return apr_get_os_error();
   }
  -
  
  
  
  1.16      +1 -1      apr/threadproc/win32/signals.c
  
  Index: signals.c
  ===================================================================
  RCS file: /home/cvs/apr/threadproc/win32/signals.c,v
  retrieving revision 1.15
  retrieving revision 1.16
  diff -u -r1.15 -r1.16
  --- signals.c	2001/02/25 20:39:40	1.15
  +++ signals.c	2001/09/09 06:03:05	1.16
  @@ -66,7 +66,7 @@
   /* Windows only really support killing process, but that will do for now. */
   APR_DECLARE(apr_status_t) apr_proc_kill(apr_proc_t *proc, int signal)
   {
  -    if (TerminateProcess((HANDLE)proc->pid, signal) == 0) {
  +    if (TerminateProcess((HANDLE)proc->hproc, signal) == 0) {
           return apr_get_os_error();
       }
       return APR_SUCCESS;
  
  
  

Re: cvs commit: apr/threadproc/win32 proc.c signals.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> wrowe       01/09/08 23:03:05
> 
>   Modified:    .        CHANGES
>                file_io/netware filesys.c
>                file_io/os2 filesys.c
>                file_io/unix filepath.c
>                file_io/win32 filepath.c filesys.c
>                include  apr_file_info.h apr_thread_proc.h
>                include/arch/os2 fileio.h
>                include/arch/win32 fileio.h threadproc.h
>                test     testmmap.c
>                threadproc/win32 proc.c signals.c
>   Log:
>     Fix the apr_proc_create for win32.  In order to do so, this patch
>     introduces a flags arg for apr_filepath_get(), like apr_filepath_merge(),
>     that allows the APR_FILEPATH_NATIVE result format.
>   
>     This launches win32 processes with the Unicode semantics (although it
>     runs sbcs apps equally well) and changes the default to 'not detached',
>     in sync with the unix default.
>   
>     Finally, assures apr_filepath_get() uses the '/' semantics on OS2 by
>     default.

Footnote;

This patch breaks all except .exe cgi's for win32, since we are passing multiple
arguments via the run_cgi_child call's command argument, instead of properly
filling in argv.  Since that's a bug, I've gone ahead and committed proper code
for win32's apr functions, and I'll go back to deal with the cgi issue tommorow.

Ryan pointed out that you can't pass multiple args for 'command'.  This turns
out to be probably untrue.  _But_ suexec won't handle them, so it's probably
safest to do this 'the proper way', always.

Bill


Re: cvs commit: apr/threadproc/win32 proc.c signals.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> wrowe       01/09/08 23:03:05
> 
>   Modified:    .        CHANGES
>                file_io/netware filesys.c
>                file_io/os2 filesys.c
>                file_io/unix filepath.c
>                file_io/win32 filepath.c filesys.c
>                include  apr_file_info.h apr_thread_proc.h
>                include/arch/os2 fileio.h
>                include/arch/win32 fileio.h threadproc.h
>                test     testmmap.c
>                threadproc/win32 proc.c signals.c
>   Log:
>     Fix the apr_proc_create for win32.  In order to do so, this patch
>     introduces a flags arg for apr_filepath_get(), like apr_filepath_merge(),
>     that allows the APR_FILEPATH_NATIVE result format.
>   
>     This launches win32 processes with the Unicode semantics (although it
>     runs sbcs apps equally well) and changes the default to 'not detached',
>     in sync with the unix default.
>   
>     Finally, assures apr_filepath_get() uses the '/' semantics on OS2 by
>     default.

Footnote;

This patch breaks all except .exe cgi's for win32, since we are passing multiple
arguments via the run_cgi_child call's command argument, instead of properly
filling in argv.  Since that's a bug, I've gone ahead and committed proper code
for win32's apr functions, and I'll go back to deal with the cgi issue tommorow.

Ryan pointed out that you can't pass multiple args for 'command'.  This turns
out to be probably untrue.  _But_ suexec won't handle them, so it's probably
safest to do this 'the proper way', always.

Bill