ATL creates wrong registry entries

[Cihan Altinay]

Hi,

For example instead of creating the key
  HKCR\CLSID{63623F01-D9C7-11D5-A76B-00008657580F}
I get
  HKCR\CLSID{
and
  HKCR\CLSID\63623F01-D9C7-11D5-A76B-00008657580F}

Maybe the trace output from atl helps finding the reason:

<snip>
trace:atl:string_register (0x4038bf40 L"HKCR\r\n{\r\n\tPSRServe.PSRRefMatrix.1 =
s 'PSRRefMatrix Class'\r\n\t{\r\n\t\tCLSID = s
'{63623F01-D9C7-11D5-A76B-00008657580F}'\r\n\t}\r\n\
tPSRServe.PSRRefMatrix = s 'PSRRefMatrix Class'\r\n\t{\r\n\t\tCLSID = s
'{63623F01-D9C7-11D5-A76B-00008657580F}'\r\n\t\tCurVer = s
'PSRServe.PSRRefMatrix.1'\r\n\t"... 1)
trace:atl:do_preprocess L"HKCR\r\n{\r\n\tPSRServe.PSRRefMatrix.1 = s
'PSRRefMatrix Class'\r\n\t{\r\n\t\tCLSID = s
'{63623F01-D9C7-11D5-A76B-00008657580F}'\r\n\t}\r\n\tPSRServe.PSRR
efMatrix = s 'PSRRefMatrix Class'\r\n\t{\r\n\t\tCLSID = s
'{63623F01-D9C7-11D5-A76B-00008657580F}'\r\n\t\tCurVer = s
'PSRServe.PSRRefMatrix.1'\r\n\t"...
trace:atl:do_process_key name = L"PSRServe.PSRRefMatrix.1"
trace:atl:do_process_key name = L"CLSID"
trace:atl:do_process_key 1 0
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"PSRServe.PSRRefMatrix"
trace:atl:do_process_key name = L"CLSID"
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"CurVer"
trace:atl:do_process_key 1 0
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"CLSID"
trace:atl:do_process_key name = L"{"
trace:atl:do_process_key 1 3
trace:atl:do_process_key name = L"63623F01-D9C7-11D5-A76B-00008657580F}"
trace:atl:do_process_key name = L"ProgID"
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"VersionIndependentProgID"
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"Programmable"
trace:atl:do_process_key 1 3
trace:atl:do_process_key name = L"LocalServer32"
trace:atl:do_process_key 1 0
trace:atl:do_process_key name = L"AppID"
trace:atl:do_process_key 1 2
trace:atl:do_process_key name = L"TypeLib"
trace:atl:do_process_key 1 0
trace:atl:do_process_key 1 0
trace:atl:do_process_key 1 1
trace:atl:Registrar_Release (0x4038bf40) ->0
</snip>

Cheers,
Cihan

[Cihan Altinay]

Cihan Altinay wrote:
> Hi,
>
> For example instead of creating the key
>   HKCR\CLSID{63623F01-D9C7-11D5-A76B-00008657580F}
> I get
>   HKCR\CLSID{
> and
>   HKCR\CLSID\63623F01-D9C7-11D5-A76B-00008657580F}
>
> Maybe the trace output from atl helps finding the reason:

Ok, I looked into it again and found a solution. The problem
was that the '{' character was treated as a separator which
is generally correct. But it seems that key names like the one
mentioned above are allowed without single quotes around them.
The attached patch doesn't look very nice but solves the problem.
I am aware that it implies that there must be whitespace between
'{' and the next character if it _is_ a separator. But it seems
the same condition is already true for '}' in the code.
Could Jacek or somebody else who worked on this file comment on
this and tell me if I can submit the patch?

Thanks,
Cihan

[Jacek Caban]

Hi Cihan,

Cihan Altinay wrote:
> Ok, I looked into it again and found a solution. The problem
> was that the '{' character was treated as a separator which
> is generally correct. But it seems that key names like the one
> mentioned above are allowed without single quotes around them.
> The attached patch doesn't look very nice but solves the problem.
> I am aware that it implies that there must be whitespace between
> '{' and the next character if it _is_ a separator. But it seems
> the same condition is already true for '}' in the code.
> Could Jacek or somebody else who worked on this file comment on
> this and tell me if I can submit the patch?

You're right. I've tested it and '{' followed by non-whitespace should
be interpreted as string, not a separator. Your patch is correct, but
removing *iter == '{' test from if has the same result and is cleaner.
Then we need to make sure that we have a single '{' character in one
more place. Could you test the attached patch?

Thanks,
    Jacek

Index: dlls/atl/registrar.c
===================================================================
RCS file: /home/wine/wine/dlls/atl/registrar.c,v
retrieving revision 1.15
diff -u -p -r1.15 registrar.c
--- dlls/atl/registrar.c	12 Sep 2005 20:29:16 -0000	1.15
+++ dlls/atl/registrar.c	16 Nov 2005 15:58:25 -0000
@@ -125,7 +125,7 @@ static HRESULT get_word(LPCOLESTR *str, 
         return S_OK;
     }

-    if(*iter == '{' || *iter == '}' || *iter == '=') {
+    if(*iter == '}' || *iter == '=') {
         strbuf_write(iter++, buf, 1);
     }else if(*iter == ''') {
         iter2 = ++iter;
@@ -321,7 +321,7 @@ static HRESULT do_process_key(LPCOLESTR 
             break;
         }

-        if(key_type != IS_VAL && key_type != DO_DELETE && *iter == '{') {
+        if(key_type != IS_VAL && key_type != DO_DELETE && *iter == '{' && !iter[1]) {
             hres = get_word(&iter, buf);
             if(FAILED(hres))
                 break;

[Cihan Altinay]

Jacek Caban wrote:
> Hi Cihan,
>
> Cihan Altinay wrote:
> > Ok, I looked into it again and found a solution. The problem
> > was that the '{' character was treated as a separator which
> > is generally correct. But it seems that key names like the one
> > mentioned above are allowed without single quotes around them.
> > The attached patch doesn't look very nice but solves the problem.
> > I am aware that it implies that there must be whitespace between
> > '{' and the next character if it _is_ a separator. But it seems
> > the same condition is already true for '}' in the code.
> > Could Jacek or somebody else who worked on this file comment on
> > this and tell me if I can submit the patch?
>
> You're right. I've tested it and '{' followed by non-whitespace should
> be interpreted as string, not a separator. Your patch is correct, but
> removing *iter == '{' test from if has the same result and is cleaner.
> Then we need to make sure that we have a single '{' character in one
> more place. Could you test the attached patch?
> @@ -321,7 +321,7 @@ static HRESULT do_process_key(LPCOLESTR 
>              break;
>          }
> -
>    if(key_type != IS_VAL && key_type != DO_DELETE && *iter == '{') {
> -
>    if(key_type != IS_VAL && key_type != DO_DELETE && *iter == '{' && !iter[1]) {
>        hres = get_word(&iter, buf);
>        if(FAILED(hres))
>            break;

if you replace !iter[1] by isspaceW(iter[1]) it works. Otherwise it
breaks things :-)

Thanks a lot,
Cihan

[Jacek Caban]

Cihan Altinay wrote:
> Jacek Caban wrote:
> > @@ -321,7 +321,7 @@ static HRESULT do_process_key(LPCOLESTR 
> >              break;
> >          }
> > -
> >    if(key_type != IS_VAL && key_type != DO_DELETE && *iter == 
> >
> > '{') {
> > -
> >    if(key_type != IS_VAL && key_type != DO_DELETE && *iter == 
> >
> > '{' && !iter[1]) {
> >              hres = get_word(&iter, buf);
> >              if(FAILED(hres))
> >                  break;
>
> if you replace !iter[1] by isspaceW(iter[1]) it works. Otherwise it
> breaks things :-)

You're right, it's my bug.

Thanks,
    Jacek