Remove redundant functions my_strncpy() and my_strncat()

components/crash/app/breakpad_linux.cc

Index: components/crash/app/breakpad_linux.cc
diff --git a/components/crash/app/breakpad_linux.cc b/components/crash/app/breakpad_linux.cc
index 393c60492f953459e26d4f643b15c71b3dcbcee5..7089a1916b749afa14460af98433c3fa7fa44522 100644
--- a/components/crash/app/breakpad_linux.cc
+++ b/components/crash/app/breakpad_linux.cc
@@ -166,35 +166,6 @@ void my_uint64tos(char* output, uint64_t i, unsigned i_len) {
     output[index - 1] = '0' + (i % 10);
 }
 
-#if defined(OS_ANDROID)
-char* my_strncpy(char* dst, const char* src, size_t len) {
-  int i = len;
-  char* p = dst;
-  if (!dst || !src)
-    return dst;
-  while (i != 0 && *src != '\0') {
-    *p++ = *src++;
-    i--;
-  }
-  while (i != 0) {
-    *p++ = '\0';
-    i--;
-  }
-  return dst;
-}
-
-char* my_strncat(char *dest, const char* src, size_t len) {
-  char* ret = dest;
-  while (*dest)
-      dest++;
-  while (len--)
-    if (!(*dest++ = *src++))
-      return ret;
-  *dest = 0;
-  return ret;
-}
-#endif
-
 #if !defined(OS_CHROMEOS)
 bool my_isxdigit(char c) {
   return (c >= '0' && c <= '9') || ((c | 0x20) >= 'a' && (c | 0x20) <= 'f');
@@ -1542,39 +1513,36 @@ void HandleCrashDump(const BreakpadInfo& info) {
 
 #if defined(OS_ANDROID)
   if (info.filename) {
-    int filename_length = my_strlen(info.filename);
+    size_t filename_length = my_strlen(info.filename);
 
     // If this was a file, we need to copy it to the right place and use the
     // right file name so it gets uploaded by the browser.
     const char msg[] = "Output crash dump file:";
     WriteLog(msg, sizeof(msg) - 1);
-    WriteLog(info.filename, filename_length - 1);

[hashimoto, 2015/06/05 07:02:43]

WriteLog's Android implementation ignores the second parameter, but it should be
nice to do it right.

+    WriteLog(info.filename, filename_length);
 
     char pid_buf[kUint64StringSize];
-    uint64_t pid_str_length = my_uint64_len(info.pid);
+    size_t pid_str_length = my_uint64_len(info.pid);
     my_uint64tos(pid_buf, info.pid, pid_str_length);
+    pid_buf[pid_str_length] = 0;  // my_uint64tos() doesn't null-terminate.
 
-    // -1 because we won't need the null terminator on the original filename.
-    unsigned done_filename_len = filename_length - 1 + pid_str_length;

[hashimoto, 2015/06/05 07:02:43]

IIUC the existing code writes more bytes than allocated to |done_filename|.

[Lei Zhang, 2015/06/05 18:24:09]

Yes, it's full of fail. :(

+    size_t done_filename_len = filename_length + pid_str_length + 1;
     char* done_filename = reinterpret_cast<char*>(
         allocator.Alloc(done_filename_len));
     // Rename the file such that the pid is the suffix in order signal to other
     // processes that the minidump is complete. The advantage of using the pid
     // as the suffix is that it is trivial to associate the minidump with the
     // crashed process.
-    // Finally, note strncpy prevents null terminators from
-    // being copied. Pad the rest with 0's.
-    my_strncpy(done_filename, info.filename, done_filename_len);
-    // Append the suffix a null terminator should be added.
-    my_strncat(done_filename, pid_buf, pid_str_length);
+    my_strlcpy(done_filename, info.filename, done_filename_len);
+    my_strlcat(done_filename, pid_buf, done_filename_len);
     // Rename the minidump file to signal that it is complete.
     if (rename(info.filename, done_filename)) {
       const char failed_msg[] = "Failed to rename:";
       WriteLog(failed_msg, sizeof(failed_msg) - 1);
-      WriteLog(info.filename, filename_length - 1);
+      WriteLog(info.filename, filename_length);
       const char to_msg[] = "to";
       WriteLog(to_msg, sizeof(to_msg) - 1);
-      WriteLog(done_filename, done_filename_len - 1);
+      WriteLog(done_filename, done_filename_len);

[Lei Zhang, 2015/06/05 18:24:09]

The -1 should remain here, right?

[hashimoto, 2015/06/06 02:11:05]

Ugh, you're right.
Fixed.

     }
   }
 #endif