On 04/16/12 22:03, Morten Rønne wrote:
Add a new common module information struct for urlcache and functions to do initialization of the struct. This is intended to be a new way to more easily create Ansi and Unicode version of the functions, by doing a Ansi/Unicode preamble code that calls a common worker function that do the real work.
Is there any reason not to call Ansi function inside Unicode function?
(This is the other way then it is done in most parts of wine, the reason for this is that urlcache stores ansi strings internally. Unicode urls should probably be converted using punycode encoding (IdnToAscii?)).
This patch is also introducing lots of unneeded Ansi/Unicode conversions. I'm not sure if storing all url entry related information inside a structure makes anything easier/more readable. Especially when you need to make sure that a field is initialized before accessing it.
Den 18-04-2012 17:42, Piotr Caban skrev:
On 04/16/12 22:03, Morten Rønne wrote:
Add a new common module information struct for urlcache and functions to do initialization of the struct. This is intended to be a new way to more easily create Ansi and Unicode version of the functions, by doing a Ansi/Unicode preamble code that calls a common worker function that do the real work.
Is there any reason not to call Ansi function inside Unicode function?
(This is the other way then it is done in most parts of wine, the reason for this is that urlcache stores ansi strings internally. Unicode urls should probably be converted using punycode encoding (IdnToAscii?)).
This patch is also introducing lots of unneeded Ansi/Unicode conversions. I'm not sure if storing all url entry related information inside a structure makes anything easier/more readable. Especially when you need to make sure that a field is initialized before accessing it.
Hi
By creating a common internal function for the Ansi and Unicode function you can use return to exit "early" in the function. Together with the common struct, this makes cleanup easy regardless of how little or how much work was done. All "tracking" of allocation is done in that one struct.
This means that most top level functions can be written: if (AcquireIndex) do work ReleaseIndex
Also from a TRACE point of view you only get one trace entry per call, with clear information whether an ansi or unicode function was called. Also by having a common function it is possible to reuse functions, e.g. RetrieveUrlCacheEntryStream can use RetrieveUrlCacheEntry common function in its implementation. This can be done because the locking/release is done once in the top level entry function.
Doing a general rule of Unicode calls Ansi function, you will end up with even more round trip conversions, because Unicode function would convert to ansi, and the ansi function would convert back to unicode in some cases, at least as the code is now. If you are to avoid this, you have to rewrite all the internal functions to use ansi only in its processing (see comment about prefix below).
Regarding Ansi/Unicode conversions. If you read the current code, you will find that certain functions will do a round trip conversion between Ansi/Unicode already. My code will only do one conversion ever, once it is fully implemented.
The current implementation requires both the unicode version and the ansi version (at least of the url), because the url prefix in the container struct, used to find the container is in unicode and the internal value used inside the cache is ansi. The same is true about all file processing which is done in unicode.
Regarding the actual conversion of ansi/unicode I have simply done what is already being done in the module. If there is another way, feel free to send the exact code that I should use or send a patch after.
Creating a few common functions will ensure that all variables needed are initialized properly, so there is no need for testing whether or not a value is filled or not.
I believe that what I propose here will make the implementation of functions simpler and safer.
Best Regards Morten Rønne
-
Morten Rønne -
Piotr Caban