You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4php-dev@logging.apache.org by Christian Hammers <ch...@lathspell.de> on 2009/10/06 21:39:25 UTC
Now come code changes
Hello
My remaining changes affect actual php code so please start having an
eye on the commit message! :)
bye,
-christian-
Re: Now come code changes
Posted by Christian Hammers <ch...@lathspell.de>.
Hello
I checked all @func() and fixed most.
There is one FIXME left that was added by me so I don't know how to fix
it.
There are print_r, var_dump and var_export but they seem to be ok.
There are many TODOs but I think if we try to fix them all we will
never release... :)
bye,
-christian-
Am Thu, 8 Oct 2009 06:39:12 +0200
schrieb Christian Grobmeier <gr...@gmail.com>:
> > I grepped for other occurances:
>
> hm.... interesting. I also think we need to grep for FIXME and TODO
> and even for echo, print_r or var_dump. Just to make sure there is
> nothing open for the release :-)
>
> Christian
>
>
> >
> > $ egrep -rsn '@[_a-zA-Z0-9]+\(' . |grep -v .svn
> > ./main/php/LoggerLoggingEvent.php:167:
> > $className =
> > @strtolower($hop['class']); ./main/php/LoggerLoggingEvent.php:169:
> > @strtolower(get_parent_class($className))
> > == 'logger' or ./main/php/LoggerLoggingEvent.php:170:
> > @strtolower(get_parent_class($className))
> > == 'loggercategory')) {
> >
> > FIXME: No clue why strtolower() could ever possibly throw a warning.
> > Even with null, false or a resource it works.
> > get_parent_class() seems also not to throw any warnings.
> >
> > ./main/php/configurators/LoggerConfiguratorIni.php:296:
> > $properties = @parse_ini_file($url);
> >
> > Warnings are catched and converted to Exception.
> >
> > ./main/php/appenders/LoggerAppenderSocket.php:108:
> > $this->sp = @fsockopen($this->getRemoteHost(),
> > $this->getPort(), $errno, $errstr, $this->getTimeout());
> >
> > FIXME: the errors are checked but there is no warning to the user.
> > I have a look at that!
> >
> > ./main/php/appenders/LoggerAppenderMailEvent.php:157:
> > @mail($to, $this->getSubject(),
> >
> > FIXME: This, too, looks suspicous. I have a look at that as well.
> >
> > ./main/php/helpers/LoggerOptionConverter.php:168: $result =
> > @call_user_func(array($clazz, 'toLevel'), $levelName,
> > $defaultValue);
> >
> > If an error occurs in toLevel() the default is returned. That's at
> > least how its documented.
> >
> >
> > bye,
> >
> > -christian-
> >
> >
Re: Now come code changes
Posted by Christian Grobmeier <gr...@gmail.com>.
> I grepped for other occurances:
hm.... interesting. I also think we need to grep for FIXME and TODO
and even for echo, print_r or var_dump. Just to make sure there is
nothing open for the release :-)
Christian
>
> $ egrep -rsn '@[_a-zA-Z0-9]+\(' . |grep -v .svn
> ./main/php/LoggerLoggingEvent.php:167: $className = @strtolower($hop['class']);
> ./main/php/LoggerLoggingEvent.php:169: @strtolower(get_parent_class($className)) == 'logger' or
> ./main/php/LoggerLoggingEvent.php:170: @strtolower(get_parent_class($className)) == 'loggercategory')) {
>
> FIXME: No clue why strtolower() could ever possibly throw a warning.
> Even with null, false or a resource it works.
> get_parent_class() seems also not to throw any warnings.
>
> ./main/php/configurators/LoggerConfiguratorIni.php:296: $properties = @parse_ini_file($url);
>
> Warnings are catched and converted to Exception.
>
> ./main/php/appenders/LoggerAppenderSocket.php:108: $this->sp = @fsockopen($this->getRemoteHost(), $this->getPort(), $errno, $errstr, $this->getTimeout());
>
> FIXME: the errors are checked but there is no warning to the user. I have a look at that!
>
> ./main/php/appenders/LoggerAppenderMailEvent.php:157: @mail($to, $this->getSubject(),
>
> FIXME: This, too, looks suspicous. I have a look at that as well.
>
> ./main/php/helpers/LoggerOptionConverter.php:168: $result = @call_user_func(array($clazz, 'toLevel'), $levelName, $defaultValue);
>
> If an error occurs in toLevel() the default is returned. That's at least how its documented.
>
>
> bye,
>
> -christian-
>
>
Re: Now come code changes
Posted by Christian Hammers <ch...@lathspell.de>.
Am Wed, 7 Oct 2009 06:50:50 +0200
schrieb Christian Grobmeier <gr...@gmail.com>:
> Yes will have
> Just one thing, I don't think its good to use @ before commands, for
> ex @mail(....)
>
> If one doesn't want errors, he should have error_reporting(0).
> Otherwirse we surpress the errors even when we want em. Saw it only
> one time in testcase, but wanted to mention
Agreed, log4php doing simply nothing without telling my why was the main
reason I started filing bugs after all :)
It's used in LoggerAppenderRollingFileTest.php for thinks like @unlink() to
remove left over tmp files which might or might not be present.
I grepped for other occurances:
$ egrep -rsn '@[_a-zA-Z0-9]+\(' . |grep -v .svn
./main/php/LoggerLoggingEvent.php:167: $className = @strtolower($hop['class']);
./main/php/LoggerLoggingEvent.php:169: @strtolower(get_parent_class($className)) == 'logger' or
./main/php/LoggerLoggingEvent.php:170: @strtolower(get_parent_class($className)) == 'loggercategory')) {
FIXME: No clue why strtolower() could ever possibly throw a warning.
Even with null, false or a resource it works.
get_parent_class() seems also not to throw any warnings.
./main/php/configurators/LoggerConfiguratorIni.php:296: $properties = @parse_ini_file($url);
Warnings are catched and converted to Exception.
./main/php/appenders/LoggerAppenderSocket.php:108: $this->sp = @fsockopen($this->getRemoteHost(), $this->getPort(), $errno, $errstr, $this->getTimeout());
FIXME: the errors are checked but there is no warning to the user. I have a look at that!
./main/php/appenders/LoggerAppenderMailEvent.php:157: @mail($to, $this->getSubject(),
FIXME: This, too, looks suspicous. I have a look at that as well.
./main/php/helpers/LoggerOptionConverter.php:168: $result = @call_user_func(array($clazz, 'toLevel'), $levelName, $defaultValue);
If an error occurs in toLevel() the default is returned. That's at least how its documented.
bye,
-christian-
Re: Now come code changes
Posted by Christian Grobmeier <gr...@gmail.com>.
Yes will have
Just one thing, I don't think its good to use @ before commands, for ex
@mail(....)
If one doesn't want errors, he should have error_reporting(0).
Otherwirse we surpress the errors even when we want em. Saw it only
one time in testcase, but wanted to mention
On Tue, Oct 6, 2009 at 9:39 PM, Christian Hammers <ch...@lathspell.de> wrote:
> Hello
>
> My remaining changes affect actual php code so please start having an
> eye on the commit message! :)
>
> bye,
>
> -christian-
>
>