You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/06/02 06:55:44 UTC
[GitHub] [logging-log4cxx] ams-tschoening commented on a diff in pull request #111: Logcxx 556
ams-tschoening commented on code in PR #111:
URL: https://github.com/apache/logging-log4cxx/pull/111#discussion_r887619267
##########
src/main/cpp/syslogappender.cpp:
##########
@@ -326,11 +327,17 @@ void SyslogAppender::append(const spi::LoggingEventPtr& event, Pool& p)
while ( start != msg.end() )
{
- LogString::iterator end = start + maxMessageLength - 12;
+ LogString::iterator end;// = start + maxMessageLength - 12;
+
+// if ( end > msg.end() )
+// {
+// end = msg.end();
+// }
- if ( end > msg.end() )
- {
+ if( msg.size() > (maxMessageLength - MINIMUM_MESSAGE_SIZE) ) {
Review Comment:
`msg.size() > (maxMessageLength - MINIMUM_MESSAGE_SIZE)` seems to not change within the loop, so put it above to declaration of `end`?
##########
src/main/cpp/syslogappender.cpp:
##########
@@ -326,11 +327,17 @@ void SyslogAppender::append(const spi::LoggingEventPtr& event, Pool& p)
while ( start != msg.end() )
{
- LogString::iterator end = start + maxMessageLength - 12;
+ LogString::iterator end;// = start + maxMessageLength - 12;
+
+// if ( end > msg.end() )
+// {
+// end = msg.end();
+// }
- if ( end > msg.end() )
- {
+ if( msg.size() > (maxMessageLength - MINIMUM_MESSAGE_SIZE) ) {
Review Comment:
`maxMessageLength - MINIMUM_MESSAGE_SIZE` looks like some sort of `msgWindowLen[gth]`? Doesn't seem to change, so might be maintained on instance level, calculated with `setMaxMessageLength`. Makes code easier to understand in my opinion, like with removing `12` in favour of some name.
##########
src/main/cpp/syslogappender.cpp:
##########
@@ -326,11 +327,12 @@ void SyslogAppender::append(const spi::LoggingEventPtr& event, Pool& p)
while ( start != msg.end() )
{
- LogString::iterator end = start + maxMessageLength - 12;
+ LogString::iterator end;
- if ( end > msg.end() )
- {
+ if( msg.size() > (maxMessageLength - MINIMUM_MESSAGE_SIZE) ) {
Review Comment:
Line 349 contains the following line which might be changed to using `MINIMUM_MESSAGE_SIZE` instead:
char buf[12];
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org