httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@rowe-clan.net
Subject RE: Re: svn commit: r1611169 - in /httpd/httpd/trunk: CHANGES server/mpm/winnt/service.c
Date Wed, 14 Jan 2015 23:20:32 GMT
--------- Original Message --------- Subject: Re: svn commit: r1611169 - in /httpd/httpd/trunk:
CHANGES server/mpm/winnt/service.c
From: "Mike Rumph" <mike.rumph@oracle.com>
Date: 1/14/15 12:24 pm
To: dev@httpd.apache.org

Hello Bill,

 Sorry to respond to this so late, but I see that this is up for a vote 
 against httpd 2.4. 
  
We love any code review, early, late, post-release, whatever!  Thanks for looking...
 
 I noticed a possible problem in the code.
 I have a question inserted below.

> + SC_HANDLE schService;
 > +
 > +#if APR_HAS_UNICODE_FS
 > + IF_WIN_OS_IS_UNICODE
 > + {
 > + schService = OpenServiceW(schSCManager,
 > + (LPCWSTR)mpm_service_name_w,
 > + SERVICE_CHANGE_CONFIG);
 > + }
 > +#endif /* APR_HAS_UNICODE_FS */
 > +#if APR_HAS_ANSI_FS
 > + ELSE_WIN_OS_IS_ANSI
 > + {
 > + schService = OpenService(schSCManager, mpm_service_name,
 > + SERVICE_CHANGE_CONFIG);
 > + }
 > +#endif
 > if (schService) {
 If neither of these defines are set, then schService appears to be 
 uninitialized.
 Is this a problem?
  
No, it cannot be an issue, nor are they mutually exclusive.  One or both are always defined.
 I trust you didn't trip over this during compilation; good eyes though :)  See also os/win32/ap_regkey.c
for much more code that follows this pattern.
 
This is the pattern behind all APR (and httpd) portability from win98 API to winNT (Unicode).
 For example, the internal representation of an apr_dir_t;
#if APR_HAS_UNICODE_FS
 struct {
 WIN32_FIND_DATAW *entry;
 } w;
#endif
#if APR_HAS_ANSI_FS
 struct {
 WIN32_FIND_DATAA *entry;
 } n;
#endif
 We have no members, w, or n, and therefore no directory entries, if one of these defines
has not been set.
 
apr_arch_misc.h contains these two macros.  The pattern exists because it was possible to
use the same binary on either OS.   IF_WIN_OS_IS_UNICODE and ELSE_WIN_OS_IS_ANSI handled the
dynamic behavior, if only one is set, those macros are a no-op.  If both are set, those macros
become if-else cases (against the version of Windows in use).
 
With Win9x dead, it is past time to prune these, at least on trunk and apr-2.0.  
 
I'm not sure whether users are building the 'ANSI-only' flavor for use on modern Windows OS
(as all 8bit chars devolve to the OEM code page of the installed version of Windows).
 
So I left well enough alone for now (no harm to users moving from 2.4.10 -> 2.4.11), and
would plan to follow whatever decisions are made on dev@apr with respect to dropping 8-bit
OEM code page logic, altogether, from trunk.
 
Bill

Mime
View raw message