Normalizing source file names before calculating hash

chrome/browser/metrics/metrics_log.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/metrics/metrics_log.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 255 matching lines @@
   while (i > 0 && isdigit(thread_name[i - 1])) {
     --i;
   }
 
   if (i == thread_name.length())
     return thread_name;
 
   return thread_name.substr(0, i) + '*';
 }
 
+// Normalizes a source filename (which is platform- and build-method-dependent)
+// by extracting the last component of the full file name.
+// Example: "c:\b\build\slave\win\build\src\chrome\app\chrome_main.cc" =>
+// "chrome_main.cc".
+std::string NormalizeFileName(const std::string& file_name) {
+  return file_name.substr(file_name.find_last_of("\\/") + 1);

[jar (doing other things), 2014/04/01 18:46:12]

I think the code works... but it probably shouldn't be trusted.

When there is no character found, you get back string::npos.

It turns out that this is usually -1.  Hence you are counting on that, and
adding one, to get 0.  The function could instead have been defined to return
-1, but that is not the API promize.

I don't believe you should count on that use of -1.

size_t offset = file_name.find_last_of("\\/");
offset = (offset == string::npos) ? 0 : offset + 1;
...

[vadimt, 2014/04/01 18:59:01]

It's not a problem for me to change the code, but "-1" is official:
http://www.cplusplus.com/reference/string/string/npos/

[jar (doing other things), 2014/04/01 19:16:17]

OK... I can live with that.  I hadn't seen it.

Thanks.

[Ilya Sherman, 2014/04/02 06:56:15]

I agree with Jim that relying on (string::npos + 1 == 0) in this way is a little
too likely to confuse people, even if it is guaranteed to be correct.  I'd
prefer to separate out the case where no file path component is found from the
case where one is.  Alternately, we could just DCHECK that one of the characters
exists in the file_name, since that should always be the case.

[vadimt, 2014/04/02 17:50:25]

Done.

+}
+
 void WriteProfilerData(const ProcessDataSnapshot& profiler_data,
                        int process_type,
                        ProfilerEventProto* performance_profile) {
   for (std::vector<tracked_objects::TaskSnapshot>::const_iterator it =
            profiler_data.tasks.begin();
        it != profiler_data.tasks.end(); ++it) {
     const tracked_objects::DeathDataSnapshot& death_data = it->death_data;
     ProfilerEventProto::TrackedObject* tracked_object =
         performance_profile->add_tracked_object();
     tracked_object->set_birth_thread_name_hash(
         MetricsLogBase::Hash(MapThreadName(it->birth.thread_name)));
     tracked_object->set_exec_thread_name_hash(
         MetricsLogBase::Hash(MapThreadName(it->death_thread_name)));
     tracked_object->set_source_file_name_hash(
-        MetricsLogBase::Hash(it->birth.location.file_name));
+        MetricsLogBase::Hash(NormalizeFileName(it->birth.location.file_name)));
     tracked_object->set_source_function_name_hash(
         MetricsLogBase::Hash(it->birth.location.function_name));
     tracked_object->set_source_line_number(it->birth.location.line_number);
     tracked_object->set_exec_count(death_data.count);
     tracked_object->set_exec_time_total(death_data.run_duration_sum);
     tracked_object->set_exec_time_sampled(death_data.run_duration_sample);
     tracked_object->set_queue_time_total(death_data.queue_duration_sum);
     tracked_object->set_queue_time_sampled(death_data.queue_duration_sample);
     tracked_object->set_process_type(AsProtobufProcessType(process_type));
     tracked_object->set_process_id(profiler_data.process_id);
@@ skipping 591 matching lines @@
     ProductDataToProto(google_update_metrics.google_update_data,
                        google_update->mutable_google_update_status());
   }
 
   if (!google_update_metrics.product_data.version.empty()) {
     ProductDataToProto(google_update_metrics.product_data,
                        google_update->mutable_client_status());
   }
 #endif  // defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
 }