Author: tschoening
Date: Tue Apr 5 15:48:18 2016
New Revision: 1737849
URL: http://svn.apache.org/viewvc?rev=1737849&view=rev
Log:
LOGCXX-464: TimeBasedRollingPolicy::initialize was able to forward an "append"
property from the caller appender, the rollover method instead wasn't and
originally used a hard coded value of "false", which was later enhanced in
LOGCXX-412 to true/false depending on some macro. This looks like an error with
the API itself to me so I changed rollover to need an append property as well.
Originally I thought of creating a backwards compatible wrapper still providing
a hard coded value of true/false depending on the macro, but because of the
abstract base RollingPolicy I needed to change that and all children anyways.
While the current approach might break callers implementing their own policy,
to me this looks like the better approach because that way those implementers
need to think of the original error and act accordingly. LOGCXX-412 shows that
some users already encountered the same error and resolved it with just another
hard coded value.
Modified:
incubator/log4cxx/trunk/src/main/cpp/fixedwindowrollingpolicy.cpp
incubator/log4cxx/trunk/src/main/cpp/rollingfileappender.cpp
incubator/log4cxx/trunk/src/main/cpp/timebasedrollingpolicy.cpp
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/fixedwindowrollingpolicy.h
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/rollingpolicy.h
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/timebasedrollingpolicy.h
Modified: incubator/log4cxx/trunk/src/main/cpp/fixedwindowrollingpolicy.cpp
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/cpp/fixedwindowrollingpolicy.cpp?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/main/cpp/fixedwindowrollingpolicy.cpp (original)
+++ incubator/log4cxx/trunk/src/main/cpp/fixedwindowrollingpolicy.cpp Tue Apr
5 15:48:18 2016
@@ -97,19 +97,22 @@ void FixedWindowRollingPolicy::activateO
* {@inheritDoc}
*/
RolloverDescriptionPtr FixedWindowRollingPolicy::initialize(
- const LogString& file, bool append, log4cxx::helpers::Pool& p) {
- LogString newActiveFile(file);
+ const LogString& currentActiveFile,
+ const bool append,
+ Pool& pool)
+{
+ LogString newActiveFile(currentActiveFile);
explicitActiveFile = false;
- if (file.length() > 0) {
+ if (currentActiveFile.length() > 0) {
explicitActiveFile = true;
- newActiveFile = file;
+ newActiveFile = currentActiveFile;
}
if (!explicitActiveFile) {
LogString buf;
ObjectPtr obj(new Integer(minIndex));
- formatFileName(obj, buf, p);
+ formatFileName(obj, buf, pool);
newActiveFile = buf;
}
@@ -122,54 +125,67 @@ RolloverDescriptionPtr FixedWindowRollin
* {@inheritDoc}
*/
RolloverDescriptionPtr FixedWindowRollingPolicy::rollover(
- const LogString& currentFileName,
- log4cxx::helpers::Pool& p) {
- RolloverDescriptionPtr desc;
- if (maxIndex >= 0) {
- int purgeStart = minIndex;
-
- if (!explicitActiveFile) {
- purgeStart++;
- }
-
- if (!purge(purgeStart, maxIndex, p)) {
- return desc;
- }
+ const LogString& currentActiveFile,
+ const bool append,
+ Pool& pool)
+{
+ RolloverDescriptionPtr desc;
+
+ if (maxIndex < 0)
+ {
+ return desc;
+ }
+
+ int purgeStart = minIndex;
+
+ if (!explicitActiveFile)
+ {
+ purgeStart++;
+ }
+
+ if (!purge(purgeStart, maxIndex, pool))
+ {
+ return desc;
+ }
+
+ LogString buf;
+ ObjectPtr obj(new Integer(purgeStart));
+ formatFileName(obj, buf, pool);
+
+ LogString renameTo(buf);
+ LogString compressedName(renameTo);
+ ActionPtr compressAction ;
+
+ if (StringHelper::endsWith(renameTo, LOG4CXX_STR(".gz")))
+ {
+ renameTo.resize(renameTo.size() - 3);
+ compressAction =
+ new GZCompressAction(
+ File().setPath(renameTo),
+ File().setPath(compressedName),
+ true);
+ }
+ else if (StringHelper::endsWith(renameTo, LOG4CXX_STR(".zip")))
+ {
+ renameTo.resize(renameTo.size() - 4);
+ compressAction =
+ new ZipCompressAction(
+ File().setPath(renameTo),
+ File().setPath(compressedName),
+ true);
+ }
+
+ FileRenameActionPtr renameAction =
+ new FileRenameAction(
+ File().setPath(currentActiveFile),
+ File().setPath(renameTo),
+ false);
+
+ desc = new RolloverDescription(
+ currentActiveFile, append,
+ renameAction, compressAction);
- LogString buf;
- ObjectPtr obj(new Integer(purgeStart));
- formatFileName(obj, buf, p);
-
- LogString renameTo(buf);
- LogString compressedName(renameTo);
- ActionPtr compressAction ;
-
- if (StringHelper::endsWith(renameTo, LOG4CXX_STR(".gz"))) {
- renameTo.resize(renameTo.size() - 3);
- compressAction =
- new GZCompressAction(
- File().setPath(renameTo), File().setPath(compressedName), true);
- } else if (StringHelper::endsWith(renameTo, LOG4CXX_STR(".zip"))) {
- renameTo.resize(renameTo.size() - 4);
- compressAction =
- new ZipCompressAction(
- File().setPath(renameTo), File().setPath(compressedName), true);
- }
-
- FileRenameActionPtr renameAction =
- new FileRenameAction(
- File().setPath(currentFileName), File().setPath(renameTo), false);
-
-#ifdef LOG4CXX_MULTI_PROCESS
- desc = new RolloverDescription(
- currentFileName, true, renameAction, compressAction);
-#else
- desc = new RolloverDescription(
- currentFileName, false, renameAction, compressAction);
-#endif
- }
-
- return desc;
+ return desc;
}
/**
Modified: incubator/log4cxx/trunk/src/main/cpp/rollingfileappender.cpp
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/cpp/rollingfileappender.cpp?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/main/cpp/rollingfileappender.cpp (original)
+++ incubator/log4cxx/trunk/src/main/cpp/rollingfileappender.cpp Tue Apr 5
15:48:18 2016
@@ -168,7 +168,7 @@ bool RollingFileAppenderSkeleton::rollov
#ifdef LOG4CXX_MULTI_PROCESS
std::string fileName(getFile());
- RollingPolicyBase *basePolicy = dynamic_cast<RollingPolicyBase*
>(&(*rollingPolicy));
+ RollingPolicyBase *basePolicy = dynamic_cast<RollingPolicyBase*
>(&(*rollingPolicy));
apr_time_t n = apr_time_now();
ObjectPtr obj(new Date(n));
LogString fileNamePattern;
@@ -185,7 +185,7 @@ bool RollingFileAppenderSkeleton::rollov
char szUid[MAX_FILE_LEN] = {'\0'};
memcpy(szDirName, fileName.c_str(), fileName.size() > MAX_FILE_LEN
? MAX_FILE_LEN : fileName.size());
memcpy(szBaseName, fileName.c_str(), fileName.size() >
MAX_FILE_LEN ? MAX_FILE_LEN : fileName.size());
- apr_uid_t uid;
+ apr_uid_t uid;
apr_gid_t groupid;
apr_status_t stat = apr_uid_current(&uid, &groupid,
pool.getAPRPool());
if (stat == APR_SUCCESS){
@@ -218,7 +218,7 @@ bool RollingFileAppenderSkeleton::rollov
if (bAlreadyRolled){
apr_finfo_t finfo1, finfo2;
apr_status_t st1, st2;
- apr_file_t* _fd =
getWriter()->getOutPutStreamPtr()->getFileOutPutStreamPtr().getFilePtr();
+ apr_file_t* _fd =
getWriter()->getOutPutStreamPtr()->getFileOutPutStreamPtr().getFilePtr();
st1 = apr_file_info_get(&finfo1, APR_FINFO_IDENT, _fd);
if (st1 != APR_SUCCESS){
LogLog::warn(LOG4CXX_STR("apr_file_info_get failed"));
@@ -229,14 +229,14 @@ bool RollingFileAppenderSkeleton::rollov
LogLog::warn(LOG4CXX_STR("apr_stat failed."));
}
- bAlreadyRolled = ((st1 == APR_SUCCESS) && (st2 == APR_SUCCESS)
+ bAlreadyRolled = ((st1 == APR_SUCCESS) && (st2 == APR_SUCCESS)
&& ((finfo1.device != finfo2.device) || (finfo1.inode !=
finfo2.inode)));
}
-
+
if (!bAlreadyRolled){
#endif
try {
- RolloverDescriptionPtr
rollover1(rollingPolicy->rollover(getFile(), p));
+ RolloverDescriptionPtr
rollover1(rollingPolicy->rollover(this->getFile(), this->getAppend(), p));
if (rollover1 != NULL) {
if (rollover1->getActiveFileName() == getFile()) {
closeWriter();
@@ -337,7 +337,7 @@ bool RollingFileAppenderSkeleton::rollov
* re-open current file when its own handler has been renamed
*/
void RollingFileAppenderSkeleton::reopenLatestFile(Pool& p){
- closeWriter();
+ closeWriter();
OutputStreamPtr os(new FileOutputStream(getFile(), true));
WriterPtr newWriter(createWriter(os));
setFile(getFile());
@@ -371,11 +371,11 @@ void RollingFileAppenderSkeleton::subApp
}
#ifdef LOG4CXX_MULTI_PROCESS
- //do re-check before every write
+ //do re-check before every write
//
apr_finfo_t finfo1, finfo2;
apr_status_t st1, st2;
- apr_file_t* _fd =
getWriter()->getOutPutStreamPtr()->getFileOutPutStreamPtr().getFilePtr();
+ apr_file_t* _fd =
getWriter()->getOutPutStreamPtr()->getFileOutPutStreamPtr().getFilePtr();
st1 = apr_file_info_get(&finfo1, APR_FINFO_IDENT, _fd);
if (st1 != APR_SUCCESS){
LogLog::warn(LOG4CXX_STR("apr_file_info_get failed"));
@@ -387,7 +387,7 @@ void RollingFileAppenderSkeleton::subApp
LogLog::warn(LOG4CXX_STR(err.c_str()));
}
- bool bAlreadyRolled = ((st1 == APR_SUCCESS) && (st2 == APR_SUCCESS)
+ bool bAlreadyRolled = ((st1 == APR_SUCCESS) && (st2 == APR_SUCCESS)
&& ((finfo1.device != finfo2.device) || (finfo1.inode != finfo2.inode)));
if (bAlreadyRolled){
@@ -524,7 +524,7 @@ size_t RollingFileAppenderSkeleton::getF
return fileLength;
}
-#ifdef LOG4CXX_MULTI_PROCESS
+#ifdef LOG4CXX_MULTI_PROCESS
void RollingFileAppenderSkeleton::setFileLength(size_t length){
fileLength = length;
}
Modified: incubator/log4cxx/trunk/src/main/cpp/timebasedrollingpolicy.cpp
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/cpp/timebasedrollingpolicy.cpp?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/main/cpp/timebasedrollingpolicy.cpp (original)
+++ incubator/log4cxx/trunk/src/main/cpp/timebasedrollingpolicy.cpp Tue Apr 5
15:48:18 2016
@@ -18,9 +18,9 @@
#pragma warning ( disable: 4231 4251 4275 4786 )
#endif
-#ifdef LOG4CXX_MULTI_PROCESS
+#ifdef LOG4CXX_MULTI_PROCESS
#include <libgen.h>
-#endif
+#endif
#include <log4cxx/logstring.h>
#include <log4cxx/rolling/timebasedrollingpolicy.h>
@@ -47,10 +47,10 @@ using namespace log4cxx::pattern;
IMPLEMENT_LOG4CXX_OBJECT(TimeBasedRollingPolicy)
-#ifdef LOG4CXX_MULTI_PROCESS
+#ifdef LOG4CXX_MULTI_PROCESS
#define MMAP_FILE_SUFFIX ".map"
#define LOCK_FILE_SUFFIX ".maplck"
-#define MAX_FILE_LEN 2048
+#define MAX_FILE_LEN 2048
bool TimeBasedRollingPolicy::isMapFileEmpty(log4cxx::helpers::Pool& pool){
apr_finfo_t finfo;
@@ -83,7 +83,7 @@ const std::string TimeBasedRollingPolicy
memcpy(szDirName, fileName.c_str(), fileName.size() > MAX_FILE_LEN ?
MAX_FILE_LEN : fileName.size());
memcpy(szBaseName, fileName.c_str(), fileName.size() > MAX_FILE_LEN ?
MAX_FILE_LEN : fileName.size());
- apr_uid_t uid;
+ apr_uid_t uid;
apr_gid_t groupid;
apr_status_t stat = apr_uid_current(&uid, &groupid, pool.getAPRPool());
if (stat == APR_SUCCESS){
@@ -138,14 +138,14 @@ int TimeBasedRollingPolicy::unLockMMapFi
#endif
-TimeBasedRollingPolicy::TimeBasedRollingPolicy()
-#ifdef LOG4CXX_MULTI_PROCESS
+TimeBasedRollingPolicy::TimeBasedRollingPolicy()
+#ifdef LOG4CXX_MULTI_PROCESS
:_mmap(NULL), _file_map(NULL), bAlreadyInitialized(false), _mmapPool(new
Pool()), _lock_file(NULL), bRefreshCurFile(false)
#endif
{
}
-#ifdef LOG4CXX_MULTI_PROCESS
+#ifdef LOG4CXX_MULTI_PROCESS
TimeBasedRollingPolicy::~TimeBasedRollingPolicy() {
//no-need to delete mmap
delete _mmapPool;
@@ -196,7 +196,7 @@ void TimeBasedRollingPolicy::activateOpt
LogLog::warn(LOG4CXX_STR("open lock file failed."));
}
}
-
+
initMMapFile(lastFileName, *_mmapPool);
#endif
@@ -226,9 +226,10 @@ log4cxx::pattern::PatternMap TimeBasedRo
* {@inheritDoc}
*/
RolloverDescriptionPtr TimeBasedRollingPolicy::initialize(
- const LogString& currentActiveFile,
- const bool append,
- Pool& pool) {
+ const LogString& currentActiveFile,
+ const bool append,
+ Pool& pool)
+{
apr_time_t n = apr_time_now();
nextCheck = ((n / APR_USEC_PER_SEC) + 1) * APR_USEC_PER_SEC;
@@ -252,11 +253,11 @@ RolloverDescriptionPtr TimeBasedRollingP
}
}
-
-
RolloverDescriptionPtr TimeBasedRollingPolicy::rollover(
- const LogString& currentActiveFile,
- Pool& pool) {
+ const LogString& currentActiveFile,
+ const bool append,
+ Pool& pool)
+{
apr_time_t n = apr_time_now();
nextCheck = ((n / APR_USEC_PER_SEC) + 1) * APR_USEC_PER_SEC;
@@ -331,17 +332,9 @@ RolloverDescriptionPtr TimeBasedRollingP
lastFileName = newFileName;
#endif
-#ifdef LOG4CXX_MULTI_PROCESS
- return new RolloverDescription(
- nextActiveFile, true, renameAction, compressAction);
-#else
- return new RolloverDescription(
- nextActiveFile, false, renameAction, compressAction);
-#endif
+ return new RolloverDescription(nextActiveFile, append, renameAction,
compressAction);
}
-
-
bool TimeBasedRollingPolicy::isTriggeringEvent(
Appender* appender,
const log4cxx::spi::LoggingEventPtr& /* event */,
Modified:
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/fixedwindowrollingpolicy.h
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/fixedwindowrollingpolicy.h?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
---
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/fixedwindowrollingpolicy.h
(original)
+++
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/fixedwindowrollingpolicy.h
Tue Apr 5 15:48:18 2016
@@ -62,8 +62,8 @@ namespace log4cxx {
* larger values are specified by the user.
*
*
- *
- *
+ *
+ *
* */
class LOG4CXX_EXPORT FixedWindowRollingPolicy : public
RollingPolicyBase {
DECLARE_LOG4CXX_OBJECT(FixedWindowRollingPolicy)
@@ -100,33 +100,21 @@ namespace log4cxx {
void setMaxIndex(int newVal);
void setMinIndex(int newVal);
-
-/**
-* Initialize the policy and return any initial actions for rolling file
appender.
-*
-* @param file current value of RollingFileAppender::getFile().
-* @param append current value of RollingFileAppender::getAppend().
-* @param p pool used for any required memory allocations.
-* @return Description of the initialization, may be null to indicate
-* no initialization needed.
-* @throws SecurityException if denied access to log files.
-*/
-virtual RolloverDescriptionPtr initialize(
-const LogString& file, const bool append, log4cxx::helpers::Pool& p);
-
-/**
-* Prepare for a rollover. This method is called prior to
-* closing the active log file, performs any necessary
-* preliminary actions and describes actions needed
-* after close of current log file.
-*
-* @param activeFile file name for current active log file.
-* @param p pool used for any required memory allocations.
-* @return Description of pending rollover, may be null to indicate no rollover
-* at this time.
-* @throws SecurityException if denied access to log files.
-*/
-virtual RolloverDescriptionPtr rollover(const LogString& activeFile,
log4cxx::helpers::Pool& p);
+ /**
+ * {@inheritDoc}
+ */
+ RolloverDescriptionPtr initialize(
+ const LogString&
currentActiveFile,
+ const bool
append,
+ log4cxx::helpers::Pool& pool);
+
+ /**
+ * {@inheritDoc}
+ */
+ RolloverDescriptionPtr rollover(
+ const LogString&
currentActiveFile,
+ const bool
append,
+ log4cxx::helpers::Pool& pool);
protected:
log4cxx::pattern::PatternMap getFormatSpecifiers() const;
Modified:
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/rollingpolicy.h
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/rollingpolicy.h?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/rollingpolicy.h
(original)
+++ incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/rollingpolicy.h
Tue Apr 5 15:48:18 2016
@@ -33,8 +33,8 @@ namespace log4cxx {
* is also responsible for providing the <em>active log file</em>,
* that is the live file where logging output will be directed.
*
- *
- *
+ *
+ *
*
*/
class LOG4CXX_EXPORT RollingPolicy :
@@ -43,35 +43,39 @@ namespace log4cxx {
public:
virtual ~RollingPolicy() {}
- /**
- * Initialize the policy and return any initial actions for rolling file
appender.
- *
- * @param file current value of RollingFileAppender.getFile().
- * @param append current value of RollingFileAppender.getAppend().
- * @param p pool for memory allocations during call.
- * @return Description of the initialization, may be null to indicate
- * no initialization needed.
- * @throws SecurityException if denied access to log files.
- */
- virtual RolloverDescriptionPtr initialize(
- const LogString& file,
- const bool append,
- log4cxx::helpers::Pool& p) = 0;
- /**
- * Prepare for a rollover. This method is called prior to
- * closing the active log file, performs any necessary
- * preliminary actions and describes actions needed
- * after close of current log file.
- *
- * @param activeFile file name for current active log file.
- * @param p pool for memory allocations during call.
- * @return Description of pending rollover, may be null to indicate no
rollover
- * at this time.
- * @throws SecurityException if denied access to log files.
- */
- virtual RolloverDescriptionPtr rollover(const LogString& activeFile,
- log4cxx::helpers::Pool& p) = 0;
+ /**
+ * Initialize the policy and return any initial actions for
rolling file appender.
+ *
+ * @param currentActiveFile current value of
RollingFileAppender.getFile().
+ * @param append current value of
RollingFileAppender.getAppend().
+ * @param pool pool for memory allocations during call.
+ * @return Description of the initialization, may be null to
indicate
+ * no initialization needed.
+ * @throws SecurityException if denied access to log files.
+ */
+ virtual RolloverDescriptionPtr initialize(
+ const LogString&
currentActiveFile,
+ const bool append,
+ log4cxx::helpers::Pool& pool) = 0;
+
+ /**
+ * Prepare for a rollover. This method is called prior to
+ * closing the active log file, performs any necessary
+ * preliminary actions and describes actions needed
+ * after close of current log file.
+ *
+ * @param currentActiveFile file name for current active log
file.
+ * @param append current value of the parent
FileAppender.getAppend().
+ * @param pool pool for memory allocations during call.
+ * @return Description of pending rollover, may be null to
indicate no rollover
+ * at this time.
+ * @throws SecurityException if denied access to log files.
+ */
+ virtual RolloverDescriptionPtr rollover(
+ const LogString&
currentActiveFile,
+ const bool append,
+ log4cxx::helpers::Pool& pool) = 0;
};
LOG4CXX_PTR_DEF(RollingPolicy);
Modified:
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/timebasedrollingpolicy.h
URL:
http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/timebasedrollingpolicy.h?rev=1737849&r1=1737848&r2=1737849&view=diff
==============================================================================
---
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/timebasedrollingpolicy.h
(original)
+++
incubator/log4cxx/trunk/src/main/include/log4cxx/rolling/timebasedrollingpolicy.h
Tue Apr 5 15:48:18 2016
@@ -246,35 +246,21 @@ namespace log4cxx {
const std::string createFile(const std::string& filename, const
std::string& suffix, log4cxx::helpers::Pool& pool);
#endif
- /**
- * Initialize the policy and return any initial actions for rolling
file appender.
- *
- * @param file current value of RollingFileAppender.getFile().
- * @param append current value of RollingFileAppender.getAppend().
- * @param pool pool for any required allocations.
- * @return Description of the initialization, may be null to indicate
- * no initialization needed.
- * @throws SecurityException if denied access to log files.
- */
- RolloverDescriptionPtr initialize(
- const LogString& file,
- const bool append,
- log4cxx::helpers::Pool& pool);
+ /**
+ * {@inheritDoc}
+ */
+ RolloverDescriptionPtr initialize(
+ const LogString&
currentActiveFile,
+ const bool
append,
+ log4cxx::helpers::Pool& pool);
- /**
- * Prepare for a rollover. This method is called prior to
- * closing the active log file, performs any necessary
- * preliminary actions and describes actions needed
- * after close of current log file.
- *
- * @param activeFile file name for current active log file.
- * @param pool pool for any required allocations.
- * @return Description of pending rollover, may be null to indicate
no rollover
- * at this time.
- * @throws SecurityException if denied access to log files.
- */
- RolloverDescriptionPtr rollover(const LogString& activeFile,
- log4cxx::helpers::Pool& pool);
+ /**
+ * {@inheritDoc}
+ */
+ RolloverDescriptionPtr rollover(
+ const LogString&
currentActiveFile,
+ const bool
append,
+ log4cxx::helpers::Pool& pool);
/**
* Determines if a rollover may be appropriate at this time. If
