[Mono-devel-list] proposed patch for System.Configuration.ConfigurationSetting

[eric lindvall]

comments inlined:

On Wed, 19 Nov 2003, Gonzalo Paniagua Javier wrote:

> El mié, 19-11-2003 a las 18:35, eric lindvall escribió:
> > sorry, forgot to cvs update first.
> 
> +		lock (typeof (DefaultConfig)) {
>  			if (instance == null) {
> -				instance = new DefaultConfig ();
> -				instance.Init ();
> +                               Interlocked.Exchange (
> +                              ref instance, new DefaultConfig());}
> -					
> 
> There's no need to use Interlocked here, as we're already inside a lock.

isn't it going to be possible that the first "instance == null" check
could find a half-assigned object ref? i debated that one for a bit, but
it seems it is "more correct" to do the exchange.


> IMO, the changes to GetDocumentForSection are not needed. I vote for
> leaving the current code as it is.

the filename check is now completely redundant, and there are cases (if
you get exceptions from within the method) where we will leak fd's if you
don't use a using() statement (or try/finally).


> Also if I try to run an application that uses ConfigurationSettings and
> there's no application configuration file (ie, only machine.config is
> read), i get an exception:
> 
> Unhandled Exception: System.IO.FileNotFoundException: Could not find
> file "/home/gpanjav/go-mono/appsettings.exe.config"
> 
> This should not happen.

correct. i'm not sure why i didn't find that when i was running my tests
(i didn't have an app.config file either).

i'll fix that.