[Mono-dev] [PATCH] New managed code for unmanaged (Win32) resourcehandling (SRE, PEAPI, MCS)

[Kornél Pál]

Hi,

Note that SRE and PEAPI parts were not discussed yet so I am waiting
comments on those parts from component owners.

> Well, I would prefer to use the memory solution.
> 1. It's faster
> 2. It's easier to implement, no problems with file removing, creating,
> locking, etc.
> 3. I don't believe that anyone will link more than 10 MB resource file.

I think this is an important feature of my resource handling infrastructure
because you can use large resources if you want. But note that FileStream
provides it's own buffering over OS file cache, so you will notice no (or
little) difference for resource files smaller than 4K as they are retained
in memory by FileStream in fact. But note that only SRE is involved in this
question as mcs does no actual resource conversion.

> 1. You don't need to use string.Format as Report.Error accepts variable
> arguments.

OK.

> 2.  Please provide error tests for as many new error codes as possible.

There are a lot of possible errors, but they are mapped to the same error
code. BTW can I place invalid .ico and .res files in errors directory where
the .cs files are?

> I prefer this method of code sharing instead of copying all the sources
> to every file. Please use reflection to get instance of your internal
> classes. I know you are using several fields, so if you really need them
> then a solution can be to introduce interface e.g. IResource with all
> required properties and methods and distribute only this interface file
> across. Of course, you will need some factory method to create this
> interface but that is the trivial part.

The Resource Table generation code is included only in SRE. Also note that
there are at least 7 classes (including DefaultResource that is the only one
directly used by mcs) that are not included in SRE as mcs generates
different default resource than SRE. And a lot of classes required by
resource converter are not included in mcs as resource conversion is
performed by SRE rather than mcs.

I belive that it's better not to extend SRE with internal methods intended
to public use if it can be achieved using public methods available in MS.NET
as well. Whats the point of doing what you suggest?

> +        private static void CreateDefaultResource (string fileName)
> Try to move this method to CodeGen.Assembly class.

OK.

> +            return ((languageId << 16) | codePage).ToString(upperCase ?
> "X8" : "x8", CultureInfo.InvariantCulture);
> Please use constants instead of X8, x8

I don't really understand what you mean on using constants.

> Please use style
>
> if (this.iconFileName == null)
>    return;

OK.

Kornél