On 6/11/07, Misha Koshelev mk144210@bcm.edu wrote:
Changes from previous version in patch 1:
- Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
- Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one "broken" (reminder: W works on XP but not 98)
- Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as it was in previous version).
- Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that ok_install does the actual install and checks that everything is installed properly, whereas test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in various ways to test effects of parameters on the installation (e.g., see patch 2)
- Use property of the "stub" functions of not changing last error to determine which function is stub instead of using registry checks
Hopefully this patchset is clearer.
This would still be clearer and more efficient if you set up a function pointer at the beginning of the tests, and just use that function for the rest of the tests. You make a helper function that tests which version (A or W) works, and assign the working version to the function pointer. This way, you don't have to deal with a useW variable, or any of that mess, and other people can write direct tests without having to use ok_install (which really shouldn't be needed if you use a function pointer). Keep in mind you'll have to use void pointers for the string parameters when declaring the function pointer type.
On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
On 6/11/07, Misha Koshelev mk144210@bcm.edu wrote:
Changes from previous version in patch 1:
- Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
- Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one "broken" (reminder: W works on XP but not 98)
- Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as it was in previous version).
- Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that ok_install does the actual install and checks that everything is installed properly, whereas test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in various ways to test effects of parameters on the installation (e.g., see patch 2)
- Use property of the "stub" functions of not changing last error to determine which function is stub instead of using registry checks
Hopefully this patchset is clearer.
This would still be clearer and more efficient if you set up a function pointer at the beginning of the tests, and just use that function for the rest of the tests. You make a helper function that tests which version (A or W) works, and assign the working version to the function pointer. This way, you don't have to deal with a useW variable, or any of that mess, and other people can write direct tests without having to use ok_install (which really shouldn't be needed if you use a function pointer). Keep in mind you'll have to use void pointers for the string parameters when declaring the function pointer type.
That sounds good but if you are just using one function pointer that could be set to either the A or W version, don't these other tests still have to compare the function pointer with each of the separate function pointers and then do any appropriate unicode or ANSI string conversions before calling it? Is this really simpler than just calling _InstallHinfSection which takes all the parameters that InstallHinfSectionA takes and does appropriate ANSI->Unicode conversion of the string if it is non-NULL (or passes it directly if it is NULL?).
I suppose if you just mean instead of useW and something like if (useW) ... else ... in _InstallHinfSection and other parts I can do if (funcptr == pInstallHinfSectionW) ... else ... then I could change this if you really think it will be clearer/easier to use for other devs. But it seems like you are saving somehow this level of complexity wouldn't be necessary, and I am not sure I see how.
Thanks Misha
On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
This would still be clearer and more efficient if you set up a function pointer at the beginning of the tests, and just use that function for the rest of the tests. You make a helper function that tests which version (A or W) works, and assign the working version to the function pointer. This way, you don't have to deal with a useW variable, or any of that mess, and other people can write direct tests without having to use ok_install (which really shouldn't be needed if you use a function pointer). Keep in mind you'll have to use void pointers for the string parameters when declaring the function pointer type.
More succinctly, what would be the advantage of a test function checking this function pointer you propose and then doing ANSI->Unicode or vice versa conversions itself over just calling the _InstallHinfSection function that already exists in my patch and does the appropriate conversions and then calls the appropriate function?
Thanks Misha
On Mon, 2007-06-11 at 23:10 -0700, James Hawkins wrote:
On 6/11/07, Misha Koshelev mk144210@bcm.edu wrote:
Changes from previous version in patch 1:
- Comments to explain why this conformance test cannot be quite standard - see Notes comment @ beginning
- Try A function first - I decided that since the A function works on Win98 but not WinXP it is the one "broken" (reminder: W works on XP but not 98)
- Added asserts in helper _InstallHinfSection function to verify A/W function pointers to make sure that in the future useW is not misassigned (currently it is assigned properly in START_TEST(install) section as it was in previous version).
- Changed test_install to ok_install to make its purpose more clear. Specifically, the idea is that ok_install does the actual install and checks that everything is installed properly, whereas test_InstallHinfSection allows us to try using the _InstallHinfSection functions (through ok_install) in various ways to test effects of parameters on the installation (e.g., see patch 2)
- Use property of the "stub" functions of not changing last error to determine which function is stub instead of using registry checks
Hopefully this patchset is clearer.
This would still be clearer and more efficient if you set up a function pointer at the beginning of the tests, and just use that function for the rest of the tests. You make a helper function that tests which version (A or W) works, and assign the working version to the function pointer. This way, you don't have to deal with a useW variable, or any of that mess, and other people can write direct tests without having to use ok_install (which really shouldn't be needed if you use a function pointer). Keep in mind you'll have to use void pointers for the string parameters when declaring the function pointer type.
You know I think you are right in that I can still make it a little simpler and I think you'll like the result better. I'll post new patches tonight.
Misha
-
James Hawkins -
Misha Koshelev