logging-log4net-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonas.Ba...@rohde-schwarz.com
Subject Re: Fix for FileAppender's TextWriter creation in OpenFile
Date Fri, 09 Sep 2016 14:57:37 GMT
Dominik Psenner <dpsenner@gmail.com> wrote on 09.09.2016 08:54:24:

> Von: Dominik Psenner <dpsenner@gmail.com>
> An: log4net-dev@logging.apache.org
> Datum: 09.09.2016 08:54
> Betreff: Re: Fix for FileAppender's TextWriter creation in OpenFile
> 
> Is 
> EventLogAppenderTest.ActivateOptionsDisablesAppenderIfSourceDoesntExist
> broken without your changes too?

The test fails also without my changes. From the comments on this special 
test case I assume the pass/fail has mainly to do with some Windows 
version and administrator privilege combinations.


> On 2016-08-22 14:10, Jonas.Baehr@rohde-schwarz.com wrote:
> Hi Dominik, 
> 
> Although there are no dedicated tests for the file appender as such,
> I did run the test suite at the time I changed the code. The recent 
> changes indtroduced with r1756284 ".NET Core improvements by Peter 
> Jas - closes #30" broke a lot of tests becuse of calling 
> `ToUpperInvariant` in string objects that can be null. I'll start 
> another thread about that issue. 
> With that fixed, only a single test 
> 
`EventLogAppenderTest.ActivateOptionsDisablesAppenderIfSourceDoesntExist`
> fails here, but this has nothing to do with the file appender change. 
> 
> Notably the RollingFileAppender tests run without issues after my 
change. 
> 
> Regards, 
> Jonas 
> 
> 
> 
> Von:        Dominik Psenner <dpsenner@apache.org> 
> An:        log4net-dev@logging.apache.org 
> Datum:        22.08.2016 13:30 
> Betreff:        Re: Fix for FileAppender's TextWriter creation in 
OpenFile 
> 
> 
> 
> Hi Jonas, 
> thanks for your patch! It looks sensible. Have you run the tests against 
it? 
> Cheers,Dominik 
> On 2016-08-22 12:27, Jonas.Baehr@rohde-schwarz.com wrote: 
> Hi, 
> 
> `FileAppender` has a vritual overload for the Method `SetQWForFiles`
> that takes a `Stream` and creates a StreamWriter from it. According 
> to the API documentation 
> """ 
> This method can be overridden by sub classes that want to wrap the 
> <see cref="Stream"/> in some way, for example to encrypt the output 
> data using a <c>System.Security.Cryptography.CryptoStream</c>.         
> """ 
> 
> Unfortunately, it doesn't get called. Instead it is inlined in 
> `OpenFile`, so derived classes have no chance in wrapping the stream
> before it is passed to the writer. 
> This trivial patch changes this. 
> 
> ---------------------8<--------------8<----------------- 
> --- C:/temp/FileAppender.cs-revBASE.svn003.tmp.cs        Sat Aug 13 
> 18:57:44 2016 
> +++ C:/Users/BAEHR/Documents/log4net-trunk/src/Appender/
> FileAppender.cs        Mon Aug 22 11:59:41 2016 
> @@ -1382 +1382 @@ namespace log4net.Appender 
> -                                                SetQWForFiles(new 
> StreamWriter(m_stream, m_encoding)); 
> +                                               
 SetQWForFiles(m_stream); 
> ---------------------8<--------------8<----------------- 
> 
> Best Regards, 
> Jonas 
> 

Mime
View raw message