[PATCH] D20572: [profile] clean up file handling code

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Tue May 24 10:53:38 PDT 2016


vsk added a comment.

Nice, thanks.


================
Comment at: lib/profile/InstrProfilingFile.c:88
@@ -82,2 +87,3 @@
   const char *Filename;
+  char *Allocated;
   FILE *File;
----------------
nit, This sounds like a name for a boolean. How about FilenameBuf?

================
Comment at: lib/profile/InstrProfilingFile.c:112
@@ -102,2 +111,3 @@
 
-static void setFilename(const char *Filename, int OwnsFilename) {
+/* Set the result of the file name parsing. If \p FilenamePat pattern is seen
+ * the first time, also truncate the file associated with that name.
----------------
This is a behavior change, right? Do we really only want to truncate profiles if the filename pattern changes?

================
Comment at: lib/profile/InstrProfilingFile.c:126
@@ +125,3 @@
+  if (NumPids)
+    strcpy(lprofCurFilename.PidChars, PidStr);
+  lprofCurFilename.NumHosts = NumHosts;
----------------
Could you use strncpy?

================
Comment at: lib/profile/InstrProfilingFile.c:129
@@ -112,1 +128,3 @@
+  if (NumHosts)
+    strcpy(lprofCurFilename.Hostname, HostStr);
 
----------------
^ Same comment

================
Comment at: lib/profile/InstrProfilingFile.c:159
@@ +158,3 @@
+            PROF_WARN(
+                "Unable to parse filename pattern %s. Use the default name.",
+                FilenamePat);
----------------
nit, Use -> Using

================
Comment at: lib/profile/InstrProfilingFile.c:168
@@ +167,3 @@
+            PROF_WARN(
+                "Unable to parse filename pattern %s. Use the default name.",
+                FilenamePat);
----------------
^ Same nit


http://reviews.llvm.org/D20572





More information about the llvm-commits mailing list