Pelagicore/ivi-logging

LogDataCommon contains possible endless loop

mbehr1 opened this issue · 4 comments

LogDataCommon constructor uses

size_t shortNamePosition = strlen(fileName) - 1;
while((shortNamePosition>=0) && (fileName[shortNamePosition] != '/') )
shortNamePosition--;

shortNamePosition should be a signed type otherwise the shortNamePosition >=0 will always be true.

proposal that optimizes the generation of the short file name only if needed and fixed the bug:
(simply call generateShortFilename() instead m_fileName)

struct LogDataCommon {
LogDataCommon(LogLevel level, const char* fileName, int lineNumber, const char* prettyFunction) {
m_level = level;
m_completeFileName = fileName;
m_fileName = NULL; // will be set on demand by calling generateShortFileName
m_lineNumber = lineNumber;
m_prettyFunction = prettyFunction;
}
LogLevel m_level;
const char* m_completeFileName;
int m_lineNumber;
const char* m_prettyFunction;
public:
const char* generateShortFileName()
{
if ((NULL==m_fileName) && m_completeFileName){
size_t shortNamePosition = strlen(m_completeFileName);
while((shortNamePosition>0) && (m_completeFileName[shortNamePosition-1] != '/') )
shortNamePosition--;
m_fileName = m_completeFileName + shortNamePosition;
}
return m_fileName;
}
protected:
const char* m_fileName;
};

Thanks a lot for your input Matthias.
Your fix has been integrated.
I believe most people would prefer to see the short file name in the logs so I did not integrate the second part of your proposal for now.

Hi Jacques,

the 2nd part does not remove the short file name.

I replaced the calls to m_fileName with calls to generateShortFileName() as well. So that everybody who doesn't want the file names doesn't get unnecessary runtime overhead.

Matthias

Am 02.06.2014 um 16:35 schrieb jacky405 notifications@github.com:

Thanks a lot for your input Matthias.
Your fix has been integrated.
I believe most people would prefer to see the short file name in the logs so I did not integrate the second part of your proposal for now.


Reply to this email directly or view it on GitHub.

Gruß

Matthias Behr

Hi Matthias,

I had my mind focused on the console backend only, which would not benefit from a lazy generation of the short file name as you suggested. But when only the DLT backend is used, you proposal definitely brings some improvement.
I have integrated your change.
Thanks