Re: advpack: Only do_ocx_reg (and thus DllRegisterServer) from RegisterOCX when 'N' is passed as a flag. Clarify documentation.
Hi James, On Thu, 13 May 2010, Gerald Pfeifer wrote: Would you feel more comfortable leaving the documentation as is and me just suggesting the following? if(strchrW(str_flags,'I')) hr = do_ocx_reg(hm, TRUE); to replace hr = do_ocx_reg(hm, TRUE); ? Or would you prefer to submit a patch to clarify the documentation (copying me) and based on that I hack the code? That way we'd ensure that the former is sufficiently clear and I assume you'll verify whether the code matches that? I just realized I did not see a response to this. How about the patch below that actually matches current documentation (which the current code does not seem to)? Gerald ChangeLog: Only register if I has been passed as a flag. --- dlls/advpack/advpack.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/dlls/advpack/advpack.c b/dlls/advpack/advpack.c index 112d38a..c57933b 100644 --- a/dlls/advpack/advpack.c +++ b/dlls/advpack/advpack.c @@ -519,7 +519,8 @@ HRESULT WINAPI RegisterOCX(HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show) if (!hm) goto done; -hr = do_ocx_reg(hm, TRUE); +if(strchrW(str_flags,'I')) +hr = do_ocx_reg(hm, TRUE); done: FreeLibrary(hm); -- 1.7.6
Re: advpack: Only do_ocx_reg (and thus DllRegisterServer) from RegisterOCX when 'N' is passed as a flag. Clarify documentation.
I recommend writing unit tests to answer the open questions. Thanks, James On Sat, Sep 24, 2011 at 2:46 PM, Gerald Pfeifer ger...@pfeifer.com wrote: Hi James, On Thu, 13 May 2010, Gerald Pfeifer wrote: Would you feel more comfortable leaving the documentation as is and me just suggesting the following? if(strchrW(str_flags,'I')) hr = do_ocx_reg(hm, TRUE); to replace hr = do_ocx_reg(hm, TRUE); ? Or would you prefer to submit a patch to clarify the documentation (copying me) and based on that I hack the code? That way we'd ensure that the former is sufficiently clear and I assume you'll verify whether the code matches that? I just realized I did not see a response to this. How about the patch below that actually matches current documentation (which the current code does not seem to)? Gerald ChangeLog: Only register if I has been passed as a flag. --- dlls/advpack/advpack.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/dlls/advpack/advpack.c b/dlls/advpack/advpack.c index 112d38a..c57933b 100644 --- a/dlls/advpack/advpack.c +++ b/dlls/advpack/advpack.c @@ -519,7 +519,8 @@ HRESULT WINAPI RegisterOCX(HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT show) if (!hm) goto done; - hr = do_ocx_reg(hm, TRUE); + if(strchrW(str_flags,'I')) + hr = do_ocx_reg(hm, TRUE); done: FreeLibrary(hm); -- 1.7.6
Re: advpack: Only do_ocx_reg (and thus DllRegisterServer) from RegisterOCX when 'N' is passed as a flag. Clarify documentation.
On Tue, 11 May 2010, James Hawkins wrote: I'm very hesitant about this. MSDN has no documentation about RegisterOCX, so I'm not sure how you're justifying this change. It's been a long time since I worked on this, so I don't remember much, but I do remember testing this method and documenting the parameters correctly. Where are you getting information that 'I' is required when using 'N'? I had to look again, and it seems the best I managed to find is the following: http://msdn.microsoft.com/en-us/library/bb759846%28VS.85%29.aspx If you experimentally verified something differently I will be happy to follow that. However, I did not found the original documentation sufficiently clear to really use it as a specification to base the implementation on. Specific questions I ran into were: - what happens if none of these are specified? - can the string contain more than one character? (pressumably yes?) - what happens if both are specified? Would you feel more comfortable leaving the documentation as is and me just suggesting the following? if(strchrW(str_flags,'I')) hr = do_ocx_reg(hm, TRUE); to replace hr = do_ocx_reg(hm, TRUE); ? Or would you prefer to submit a patch to clarify the documentation (copying me) and based on that I hack the code? That way we'd ensure that the former is sufficiently clear and I assume you'll verify whether the code matches that? Whatever works for you as long as we make progress. :-) Gerald
Re: advpack: Only do_ocx_reg (and thus DllRegisterServer) from RegisterOCX when 'N' is passed as a flag. Clarify documentation.
I'm very hesitant about this. MSDN has no documentation about
RegisterOCX, so I'm not sure how you're justifying this change. It's
been a long time since I worked on this, so I don't remember much, but
I do remember testing this method and documenting the parameters
correctly. Where are you getting information that 'I' is required
when using 'N'?
James
On Tue, May 11, 2010 at 1:24 PM, Gerald Pfeifer ger...@pfeifer.com wrote:
This is my humble attempt of addressing Alexandre's feeback at
http://www.winehq.org/pipermail/wine-devel/2010-May/083518.html
It does pass testing for me, even on FreeBSD/i386, but I will say I did
not find the MSDN documentation I located too helpful/clear, and may have
been misled.
As a next step we'd need to look into DllInstall. This change, hopefully,
is a move in the right direction, though. If not, some guidance or one of
the pros looking into it will be appreciated.
Gerald
---
dlls/advpack/advpack.c | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/dlls/advpack/advpack.c b/dlls/advpack/advpack.c
index 112d38a..f040ec6 100644
--- a/dlls/advpack/advpack.c
+++ b/dlls/advpack/advpack.c
@@ -486,8 +486,10 @@ HRESULT do_ocx_reg(HMODULE hocx, BOOL do_reg)
* NOTES
* OCX - Filename of the OCX to register.
* flags - Controls the operation of RegisterOCX.
- * 'I' Call DllRegisterServer and DllInstall.
- * 'N' Only call DllInstall.
+ * 'I' Call DllInstall and, unless 'N' is specified as well,
+ * DllRegisterServer.
+ * 'N' Do not call DllRegisterServer; only valid if 'I' is
+ * specified too.
* param - Command line passed to DllInstall.
*/
HRESULT WINAPI RegisterOCX(HWND hWnd, HINSTANCE hInst, LPCSTR cmdline, INT
show)
@@ -519,8 +521,12 @@ HRESULT WINAPI RegisterOCX(HWND hWnd, HINSTANCE hInst,
LPCSTR cmdline, INT show)
if (!hm)
goto done;
- hr = do_ocx_reg(hm, TRUE);
-
+ if(strchrW(str_flags,'I'))
+ {
+ if (!strchrW(str_flags,'N'))
+ hr = do_ocx_reg(hm, TRUE);
+ }
+
done:
FreeLibrary(hm);
HeapFree(GetProcessHeap(), 0, cmdline_copy);
--
1.6.6.2
