You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4cxx-dev@logging.apache.org by Laurent RICHARD <la...@harvest.fr> on 2004/09/22 20:30:42 UTC

Some code changes ?

Hello,

    I don't know if this is the right way to do this (I'm quite new to open
source project management and CVS and my C++ might be better than my english
;->).
    I checked out version 0.9.7 from the CVS repository and imported it in
our local CVS repository in order to adapt it to our configuration (we use
Borland C++ Builder).
    I think I found some little bugs and made some changes to the code. Here
is the result of a cvs diff that shows what I have done (revisions
correspond to my repository but it should be equivalent to the v0_9_7 tag in
yours) .

Index: src/fileappender.cpp
===================================================================
RCS file: /cvsroot/log4cxx/src/fileappender.cpp,v
retrieving revision 1.1.1.1
diff -r1.1.1.1 fileappender.cpp
166c166
<   WriterAppender::setOption(name, value);
---
>   WriterAppender::setOption(option, value);

Index: src/patternparser.cpp
===================================================================
RCS file: /cvsroot/log4cxx/src/patternparser.cpp,v
retrieving revision 1.1.1.1
diff -r1.1.1.1 patternparser.cpp
405c405
< void PatternParser::LiteralPatternConverter::format(StringBuffer& sbuf,
---
> void PatternParser::LiteralPatternConverter::format(ostream& sbuf,

Index: src/rootcategory.cpp
===================================================================
RCS file: /cvsroot/log4cxx/src/rootcategory.cpp,v
retrieving revision 1.1.1.1
diff -r1.1.1.1 rootcategory.cpp
31c31
< const LevelPtr& RootCategory::getEffectiveLevel()
---
> const LevelPtr& RootCategory::getEffectiveLevel() const

Index: src/stringtokenizer.cpp
===================================================================
RCS file: /cvsroot/log4cxx/src/stringtokenizer.cpp,v
retrieving revision 1.1.1.2
diff -r1.1.1.2 stringtokenizer.cpp
42c42
<  delete this->str;
---
>  delete[] this->str;

Index: include/log4cxx/helpers/patternparser.h
===================================================================
RCS file: /cvsroot/log4cxx/include/log4cxx/helpers/patternparser.h,v
retrieving revision 1.1.1.2
diff -r1.1.1.2 patternparser.h
102c102
<     virtual void format(StringBuffer& sbuf,
---
>     virtual void format(ostream& sbuf,

Index: include/log4cxx/spi/rootcategory.h
===================================================================
RCS file: /cvsroot/log4cxx/include/log4cxx/spi/rootcategory.h,v
retrieving revision 1.1.1.1
diff -r1.1.1.1 rootcategory.h
48c48
<             virtual const LevelPtr& getEffectiveLevel();
---
>             virtual const LevelPtr& getEffectiveLevel() const;


    Can you handle this ? Is there a better way to proceed to this kind of
submission ?
    In fact, I eventually have more changes (linked to the BCB support) to
submit to you if you find it useful.
    By the way, I almost forgot to say that we really appreciate the work
done by the log4cxx team :-)

Regards

Laurent R



Re: Some code changes ?

Posted by Laurent RICHARD <la...@harvest.fr>.
> Which version and platform of C++ Builder were you using?

We're using Borlanf C++ Builder 6 Enterprise on Windows 2000 but I guess the
changes we made apply to any BCB6 on any Win32 platform...

> Could you provide
> a short reason for each change or any observable bugs due to the
> original code?  I can guess if I have to, but it would be nice to have
> your comments.

Yes, of course. In fact, the changes I sent did not strictly concern BCB6
port for this time (I have some other changes in the pipe but I thought you
might not want them to be applied straightforward as I did and I also
thought this was enough for a first shot...).

Here are the reason for each of them :

fileappender.cpp :
The name was passed instead of the option. As far as I can remember, I
discovered this when I tried to set the threshold option (for instance) on a
FileAppender through a configuration file and I realized it was not taken
into account whereas it works for other appenders...

patternparser.h + patternparser.cpp and rootcategory.h + rootcategory.cpp :
BCB warns against methods that hide base class methods. I think these
virtual methods intended to override the one declared in the base class and
thus changed their signature in consequence. I didn't try to find any
related bug though.

src/stringtokenizer.cpp :
I changed the delete to delete[] since the allocation is done through new[].
I know you know that having unrelated new/delete can cause memory troubles
with some compilers.

Hope it helps

Regards

Laurent R



Re: Some code changes ?

Posted by Curt Arnold <ca...@houston.rr.com>.
>     Can you handle this ? Is there a better way to proceed to this 
> kind of
> submission ?
>     In fact, I eventually have more changes (linked to the BCB 
> support) to
> submit to you if you find it useful.
>     By the way, I almost forgot to say that we really appreciate the 
> work
> done by the log4cxx team :-)
>

Thanks, I can handle the patches in the form that you submitted.  Which 
version and platform of C++ Builder were you using?  Could you provide 
a short reason for each change or any observable bugs due to the 
original code?  I can guess if I have to, but it would be nice to have 
your comments.