[Lldb-commits] [lldb] r235589 - MI fix allowing multiple logging instances of lldb-mi to run simultaneously.

Ilia K ki.stfu at gmail.com
Thu Apr 23 05:48:42 PDT 2015


Author: ki.stfu
Date: Thu Apr 23 07:48:42 2015
New Revision: 235589

URL: http://llvm.org/viewvc/llvm-project?rev=235589&view=rev
Log:
MI fix allowing multiple logging instances of lldb-mi to run simultaneously.

Summary:
Currently if two instances of lldb-mi are running with logging enabled using '--log' the log file conflicts. This produces the following error 
MI: Error: File Handler. Error Permission denied opening 'C:\Users\Ewan\LLVM\build\Debug\bin\lldb-mi-log.txt'

Fixed in this patch by renaming lldb-mi-log.txt based on the date, e.g. lldb-mi-log.txt-20150316163631.log, and moving the file into the temp directory by using the --log-dir option.

Regrading previous review comments the P_tmpdir macro is defined in Windows but always points to "\", which doesn't help much. Also when using the Windows API for GetTempPath() dynamic memory seems much more messy.

Patch from ewan at codeplay.com

Reviewers: abidh, EwanCrawford

Subscribers: zturner, lldb-commits, deepak2427

Differential Revision: http://reviews.llvm.org/D9054

Modified:
    lldb/trunk/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
    lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.cpp
    lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.h
    lldb/trunk/tools/lldb-mi/MICmnResources.cpp
    lldb/trunk/tools/lldb-mi/MICmnResources.h
    lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
    lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.cpp
    lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.h
    lldb/trunk/tools/lldb-mi/MIUtilSystemWindows.cpp

Modified: lldb/trunk/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py (original)
+++ lldb/trunk/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py Thu Apr 23 07:48:42 2015
@@ -127,6 +127,79 @@ class MiStartupOptionsTestCase(lldbmi_te
 
         # Test that lldb-mi is ready when executable was loaded
         self.expect(self.child_prompt, exactly = True)
+    
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    def test_lldbmi_log_option(self):
+        """Test that 'lldb-mi --log' creates a log file in the current directory."""
+    
+        logDirectory = "."
+        self.spawnLldbMi(args = "%s --log" % self.myexe)
 
+        # Test that lldb-mi is ready after startup
+        self.expect(self.child_prompt, exactly = True)
+
+        # Test that the executable is loaded when file was specified
+        self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
+        self.expect("\^done")
+
+        # Test that lldb-mi is ready when executable was loaded
+        self.expect(self.child_prompt, exactly = True)
+
+        # Run
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"exited-normally\"")
+
+        # Check log file is created
+        import glob,os
+        logFile = glob.glob(logDirectory + "/lldb-mi-*.log")
+
+        if not logFile:
+            self.fail("log file not found")
+
+        # Delete log
+        for f in logFile:
+            os.remove(f)
+
+    @lldbmi_test
+    @expectedFailureWindows("llvm.org/pr22274: need a pexpect replacement for windows")
+    @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races
+    def test_lldbmi_log_directory_option(self):
+        """Test that 'lldb-mi --log --log-dir' creates a log file in the directory specified by --log-dir."""
+    
+        # Create log in temp directory
+        import tempfile
+        logDirectory = tempfile.gettempdir()
+
+        self.spawnLldbMi(args = "%s --log --log-dir=%s" % (self.myexe,logDirectory))
+
+        # Test that lldb-mi is ready after startup
+        self.expect(self.child_prompt, exactly = True)
+
+        # Test that the executable is loaded when file was specified
+        self.expect("-file-exec-and-symbols \"%s\"" % self.myexe)
+        self.expect("\^done")
+
+        # Test that lldb-mi is ready when executable was loaded
+        self.expect(self.child_prompt, exactly = True)
+
+        # Run
+        self.runCmd("-exec-run")
+        self.expect("\^running")
+        self.expect("\*stopped,reason=\"exited-normally\"")
+
+        # Check log file is created
+        import glob,os
+        logFile = glob.glob(logDirectory + "/lldb-mi-*.log")
+
+        if not logFile:
+            self.fail("log file not found")             
+   
+        # Delete log
+        for f in logFile:
+            os.remove(f)
+       
 if __name__ == '__main__':
     unittest2.main()

Modified: lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.cpp?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.cpp Thu Apr 23 07:48:42 2015
@@ -27,7 +27,9 @@
 //--
 CMICmnLogMediumFile::CMICmnLogMediumFile(void)
     : m_constThisMediumName(MIRSRC(IDS_MEDIUMFILE_NAME))
-    , m_constMediumFileName("lldb-mi-log.txt")
+    , m_constMediumFileNameFormat("lldb-mi-%s.log")
+    , m_strMediumFileName(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
+    , m_strMediumFileDirectory(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
     , m_fileNamePath(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH))
     , m_eVerbosityType(CMICmnLog::eLogVerbosity_Log)
     , m_strDate(CMIUtilDateTimeStd().GetDate())
@@ -72,7 +74,8 @@ CMICmnLogMediumFile::Instance(void)
 bool
 CMICmnLogMediumFile::Initialize(void)
 {
-    m_bInitialized = FileFormFileNamePath();
+    m_bInitialized = CMIUtilSystem().GetLogFilesPath(m_strMediumFileDirectory);
+    m_bInitialized &= FileFormFileNamePath();
 
     return m_bInitialized;
 }
@@ -213,21 +216,15 @@ CMICmnLogMediumFile::FileFormFileNamePat
 
     m_fileNamePath = MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH);
 
-    CMIUtilString strPathName;
-    if (CMIUtilSystem().GetLogFilesPath(strPathName))
+    if (m_strMediumFileDirectory.compare(MIRSRC(IDS_MEDIUMFILE_ERR_INVALID_PATH)) != 0)
     {
-        const CMIUtilString strPath = CMIUtilFileStd::StripOffFileName(strPathName);
+        CMIUtilDateTimeStd date;
+        m_strMediumFileName = CMIUtilString::Format(m_constMediumFileNameFormat.c_str(), date.GetDateTimeLogFilename().c_str());
 
-// ToDo: Review this LINUX log file quick fix so not hidden
-// AD:
-//      Linux was creating a log file here called '.\log.txt'.  The '.' on linux
-//      signifies that this file is 'hidden' and not normally visible.  A quick fix
-//      is to remove the path component all together.  Linux also normally uses '/'
-//      as directory separators, again leading to the problem of the hidden log.
 #if defined(_MSC_VER)
-        m_fileNamePath = CMIUtilString::Format("%s\\%s", strPath.c_str(), m_constMediumFileName.c_str());
+        m_fileNamePath = CMIUtilString::Format("%s\\%s", m_strMediumFileDirectory.c_str(), m_strMediumFileName.c_str());
 #else
-        m_fileNamePath = CMIUtilString::Format("%s", m_constMediumFileName.c_str());
+        m_fileNamePath = CMIUtilString::Format("%s/%s", m_strMediumFileDirectory.c_str(), m_strMediumFileName.c_str());
 #endif // defined ( _MSC_VER )
 
         return MIstatus::success;
@@ -261,7 +258,7 @@ CMICmnLogMediumFile::GetFileNamePath(voi
 const CMIUtilString &
 CMICmnLogMediumFile::GetFileName(void) const
 {
-    return m_constMediumFileName;
+    return m_strMediumFileName;
 }
 
 //++ ------------------------------------------------------------------------------------
@@ -426,3 +423,19 @@ CMICmnLogMediumFile::GetLineReturn(void)
 {
     return m_file.GetLineReturn();
 }
+
+//++ ------------------------------------------------------------------------------------
+// Details: Set the diretory to place the log file.
+// Type:    Method.
+// Args:    vPath   - (R) Path to log.
+// Return:  MIstatus::success - Functional succeeded.
+//          MIstatus::failure - Functional failed.
+// Throws:  None.
+//--
+bool
+CMICmnLogMediumFile::SetDirectory(const CMIUtilString &vPath)
+{
+    m_strMediumFileDirectory = vPath;
+
+    return FileFormFileNamePath();
+}

Modified: lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.h?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLogMediumFile.h Thu Apr 23 07:48:42 2015
@@ -43,6 +43,7 @@ class CMICmnLogMediumFile : public CMICm
     bool IsOk(void) const;
     bool IsFileExist(void) const;
     const CMIUtilString &GetLineReturn(void) const;
+    bool SetDirectory(const CMIUtilString &vPath);
 
     // Overridden:
   public:
@@ -71,8 +72,10 @@ class CMICmnLogMediumFile : public CMICm
     // Attributes:
   private:
     const CMIUtilString m_constThisMediumName;
-    const CMIUtilString m_constMediumFileName;
+    const CMIUtilString m_constMediumFileNameFormat;
     //
+    CMIUtilString m_strMediumFileName;
+    CMIUtilString m_strMediumFileDirectory;
     CMIUtilString m_fileNamePath;
     MIuint m_eVerbosityType;
     CMIUtilString m_strDate;

Modified: lldb/trunk/tools/lldb-mi/MICmnResources.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.cpp?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnResources.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.cpp Thu Apr 23 07:48:42 2015
@@ -64,6 +64,7 @@ const CMICmnResources::SRsrcTextData CMI
     {IDE_MI_APP_ARG_INTERPRETER, "--interpreter\n\t This option is kept for backward compatibility. This executable always run in MI mode"},
     {IDE_MI_APP_ARG_EXECUTEABLE, "--executable\n\tUse the MI Driver in MI mode for the debugging the specified executable." },
     {IDE_MI_APP_ARG_APP_LOG, "--log\n\tUse this argument to tell the MI Driver to update it's log\n\tfile '%s'."},
+    {IDE_MI_APP_ARG_APP_LOG_DIR, "--log-dir\n\tUse this argument to specify the directory the MI Driver\n\twill place the log file in, i.e --log-dir=/tmp." },
     {IDE_MI_APP_ARG_EXAMPLE, "Example MI command:\n\t3-info-gdb-mi-command gdb-set\n\t3^done,command={exists=\"true\"}"},
     {IDE_MI_APP_ARG_EXECUTABLE, "executable (NOT IMPLEMENTED)\n\tThe file path to the executable i.e. '\"C:\\My Dev\\foo.exe\"'."},
     {IDS_STDIN_ERR_INVALID_PROMPT, "Stdin. Invalid prompt description '%s'"},

Modified: lldb/trunk/tools/lldb-mi/MICmnResources.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.h?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MICmnResources.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.h Thu Apr 23 07:48:42 2015
@@ -74,6 +74,7 @@ enum
     IDE_MI_APP_ARG_INTERPRETER,
     IDE_MI_APP_ARG_EXECUTEABLE,
     IDE_MI_APP_ARG_APP_LOG,
+    IDE_MI_APP_ARG_APP_LOG_DIR,
     IDE_MI_APP_ARG_EXAMPLE,
     IDE_MI_APP_ARG_EXECUTABLE,
 

Modified: lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp Thu Apr 23 07:48:42 2015
@@ -427,6 +427,7 @@ CMIDriverMgr::DriverGetTheDebugger(void)
 //              --versionLong
 //              --log
 //              --executable
+//              --log-dir
 //          The above arguments are not handled by any driver object except for --executable.
 //          The options --interpreter and --executable in code act very similar. The
 //          --executable is necessary to differentiate whither the MI Driver is being using
@@ -436,7 +437,8 @@ CMIDriverMgr::DriverGetTheDebugger(void)
 //          Driver is being called the command line and that the executable argument is indeed
 //          a specified executable an so actions commands to set up the executable for a
 //          debug session. Using --interpreter on the commnd line does not action additional
-//          commands to initialise a debug session and so be able to launch the process.
+//          commands to initialise a debug session and so be able to launch the process. The directory
+//          where the log file is created is specified using --log-dir.
 // Type:    Method.
 // Args:    argc        - (R)   An integer that contains the count of arguments that follow in
 //                              argv. The argc parameter is always greater than or equal to 1.
@@ -483,7 +485,9 @@ CMIDriverMgr::ParseArgs(const int argc,
     bool bHaveArgVersion = false;
     bool bHaveArgVersionLong = false;
     bool bHaveArgLog = false;
+    bool bHaveArgLogDir = false;
     bool bHaveArgHelp = false;
+    CMIUtilString strLogDir;
 
     bHaveArgInterpret = true;
     if (bHaveArgs)
@@ -512,6 +516,11 @@ CMIDriverMgr::ParseArgs(const int argc,
             {
                 bHaveArgLog = true;
             }
+            if (0 == strArg.compare(0,10,"--log-dir="))
+            {
+                strLogDir = strArg.substr(10,CMIUtilString::npos);
+                bHaveArgLogDir = true;
+            }
             if ((0 == strArg.compare("--help")) || (0 == strArg.compare("-h")))
             {
                 bHaveArgHelp = true;
@@ -524,6 +533,11 @@ CMIDriverMgr::ParseArgs(const int argc,
         CMICmnLog::Instance().SetEnabled(true);
     }
 
+    if (bHaveArgLogDir)
+    {
+        bOk = bOk && CMICmnLogMediumFile::Instance().SetDirectory(strLogDir);
+    }
+
     // Todo: Remove this output when MI is finished. It is temporary to persuade Ecllipse plugin to work.
     //       Eclipse reads this literally and will not work unless it gets this exact version text.
     // Handle --version option (ignore the --interpreter option if present)
@@ -614,6 +628,7 @@ CMIDriverMgr::GetHelpOnCmdLineArgOptions
         MIRSRC(IDE_MI_APP_ARG_INTERPRETER),
         MIRSRC(IDE_MI_APP_ARG_EXECUTEABLE),
         CMIUtilString::Format(MIRSRC(IDE_MI_APP_ARG_APP_LOG), CMICmnLogMediumFile::Instance().GetFileName().c_str()),
+        MIRSRC(IDE_MI_APP_ARG_APP_LOG_DIR),
         MIRSRC(IDE_MI_APP_ARG_EXECUTABLE),
         MIRSRC(IDS_CMD_QUIT_HELP),
         MIRSRC(IDE_MI_APP_ARG_EXAMPLE)};

Modified: lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.cpp?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.cpp Thu Apr 23 07:48:42 2015
@@ -71,3 +71,21 @@ CMIUtilDateTimeStd::GetTime(void)
 
     return strTime;
 }
+
+//++ ------------------------------------------------------------------------------------
+// Details: Retrieve system local current date and time in yyyy-MM-dd--HH-mm-ss format for log file names.
+// Type:    Method.
+// Args:    None.
+// Return:  CMIUtilString - Text description.
+// Throws:  None.
+//--
+CMIUtilString
+CMIUtilDateTimeStd::GetDateTimeLogFilename(void)
+{
+    std::time(&m_rawTime);
+    const std::tm *pTi = std::localtime(&m_rawTime);
+    const CMIUtilString strTime(CMIUtilString::Format("%d%02d%02d%02d%02d%02d", pTi->tm_year + 1900, pTi->tm_mon,
+                                                      pTi->tm_mday, pTi->tm_hour, pTi->tm_min, pTi->tm_sec));
+
+    return strTime;
+}

Modified: lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.h?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.h (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilDateTimeStd.h Thu Apr 23 07:48:42 2015
@@ -30,6 +30,7 @@ class CMIUtilDateTimeStd
 
     CMIUtilString GetDate(void);
     CMIUtilString GetTime(void);
+    CMIUtilString GetDateTimeLogFilename(void);
 
     // Overrideable:
   public:

Modified: lldb/trunk/tools/lldb-mi/MIUtilSystemWindows.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilSystemWindows.cpp?rev=235589&r1=235588&r2=235589&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-mi/MIUtilSystemWindows.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilSystemWindows.cpp Thu Apr 23 07:48:42 2015
@@ -17,6 +17,7 @@
 // In-house headers:
 #include "MIUtilSystemWindows.h"
 #include "MICmnResources.h"
+#include "MIUtilFileStd.h"
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMIUtilSystemWindows constructor.
@@ -133,7 +134,8 @@ CMIUtilSystemWindows::GetExecutablesPath
 bool
 CMIUtilSystemWindows::GetLogFilesPath(CMIUtilString &vrwFileNamePath) const
 {
-    return GetExecutablesPath(vrwFileNamePath);
+    vrwFileNamePath = CMIUtilString(".");
+    return MIstatus::success;
 }
 
 #endif // #if defined( _MSC_VER )





More information about the lldb-commits mailing list