kernel32 (profile.c): current section and key implementation

[Claudio Fontana]

Hello all,

I posted previously a patch to wine-patches here:

http://www.winehq.org/pipermail/wine-patches/2006-December/034022.html

However, I now understand that although it solves the issue I had with the
specific program, the change is incomplete, since in several other places,
sections and keys are only considered meaningful when not equal to the
empty string.
In the Microsoft implementation, the unnamed section is valid, as are
zero length keys.

Thus, I think a broader scope change is required.
Before I start working on a new patch, do you acknowledge that the
current implementation
is incorrect, and a patch fixing it by not requiring name[0] to be !=
'\0' is likely to be accepted? (btw, I've noticed you switched to git,
so I will be using that instead now).

Thanks,

Claudio Fontana

[Robert Shearman]

Claudio Fontana wrote:
> Hello all,
>
> I posted previously a patch to wine-patches here:
>
> http://www.winehq.org/pipermail/wine-patches/2006-December/034022.html
>
> However, I now understand that although it solves the issue I had with 
> the
> specific program, the change is incomplete, since in several other 
> places,
> sections and keys are only considered meaningful when not equal to the
> empty string.
> In the Microsoft implementation, the unnamed section is valid, as are
> zero length keys.
>
> Thus, I think a broader scope change is required.
> Before I start working on a new patch, do you acknowledge that the
> current implementation
> is incorrect, and a patch fixing it by not requiring name[0] to be !=
> '\0' is likely to be accepted? (btw, I've noticed you switched to git,
> so I will be using that instead now).

Hi Claudio,

The patch looks good, apart from the specific version check for win95. 
Do you think you could add some tests to our test framework 
(dlls/kernel32/tests) to test for this issue that you've found. Then the 
tests could be run on a variety of platforms to see if it is only win95 
that has that specific behaviour or if the original author of the 
comment was wrong about that.

The tests that fail on Wine will then determine what needs to be fixed.

Thanks,

[Claudio Fontana]

Hello Robert,

On 12/24/06, Robert Shearman  wrote:
> Hi Claudio,
>
> The patch looks good, apart from the specific version check for win95.
> Do you think you could add some tests to our test framework
> (dlls/kernel32/tests) to test for this issue that you've found. Then the
> tests could be run on a variety of platforms to see if it is only win95
> that has that specific behaviour or if the original author of the
> comment was wrong about that.
>
> The tests that fail on Wine will then determine what needs to be fixed.
>
> Thanks,
>
> --
> Rob Shearman

Sure, here's what I came up with. I do not understand, however, what
the difference
between result and result9x in the test structure is.
Is result9x the result expected on windows 95/98?
I have set result == result9x for my test for now.

I do not know whether the comment (and code) of the original author is correct,
and have no access to a Windows 95 installation to test it (or any other Windows
version for that matter).

I do not respect the 80 columns limit in the test, since the file
already violates it.

The complete change would be (also attached):

--- dlls/kernel32/profile.c	21 Dec 2006 16:37:29 -0000	1.4
+++ dlls/kernel32/profile.c	25 Dec 2006 00:27:55 -0000
@@ -599,9 +599,8 @@

while (*section)
     {
-        if ( ((*section)->name[0])
-             && (!(strncmpiW( (*section)->name, section_name, seclen )))
-             && (((*section)->name)[seclen] == '\0') )
+	if (!strncmpiW((*section)->name, section_name, seclen) &&
+	    ((*section)->name)[seclen] == '\0')
         {
             PROFILEKEY **key = &(*section)->key;

@@ -959,11 +958,6 @@
     if (!def_val) def_val = empty_strW;
     if (key_name)
     {
-	if (!key_name[0])
-        {
-            /* Win95 returns 0 on keyname "". Tested with Likse32 bon 000227 */
-            return 0;
-        }
         key = PROFILE_Find( &CurProfile->section, section, key_name,
FALSE, FALSE);
         PROFILE_CopyEntry( buffer, (key && key->value) ? key->value : def_val,
                            len, TRUE );

--- dlls/kernel32/tests/profile.c	12 Sep 2006 12:31:37 -0000	1.1
+++ dlls/kernel32/tests/profile.c	25 Dec 2006 00:27:56 -0000
@@ -68,6 +68,8 @@
          { SECTION, KEY,  "42A94967297", TESTFILE,  1, 42         , 42},
          { SECTION, KEY,  "B4294967297", TESTFILE, -1, 0          , 0},
          { SECTION, KEY,  "B4294967297", TESTFILE,  1, 0          , 0},
+	 { "",      KEY,  "1",           TESTFILE,  0, 1          , 1}, /* 25 */
+	 { SECTION, "",   "1",           TESTFILE,  0, 1          , 1},
     };
     int i, num_test = (sizeof(profileInt)/sizeof(struct _profileInt));
     UINT res;
@@ -76,8 +78,10 @@

for (i=0; i < num_test; i++) {
         if (profileInt[i].value)
-            WritePrivateProfileStringA(SECTION, KEY, profileInt[i].value,
-                                      profileInt[i].iniFile);
+            WritePrivateProfileStringA(profileInt[i].section,
+				       profileInt[i].key,
+				       profileInt[i].value,
+				       profileInt[i].iniFile);

res = GetPrivateProfileIntA(profileInt[i].section, profileInt[i].key,
                  profileInt[i].defaultVal, profileInt[i].iniFile);

Claudio