Take ownership and close the platform file even if we fail to start logging.

webrtc/call/rtc_event_log.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 31 matching lines @@
 
 // No-op implementation if flag is not set.
 class RtcEventLogNullImpl final : public RtcEventLog {
  public:
   bool StartLogging(const std::string& file_name,
                     int64_t max_size_bytes) override {
     return false;
   }
   bool StartLogging(rtc::PlatformFile platform_file,
                     int64_t max_size_bytes) override {
+    // The platform_file is open and needs to be closed.
+    if (!rtc::ClosePlatformFile(platform_file)) {
+      LOG(LS_WARNING) << "Can't close file.";
+    }
     return false;
   }
   void StopLogging() override {}
   void LogVideoReceiveStreamConfig(
       const VideoReceiveStream::Config& config) override {}
   void LogVideoSendStreamConfig(
       const VideoSendStream::Config& config) override {}
   void LogRtpHeader(PacketDirection direction,
                     MediaType media_type,
                     const uint8_t* header,
@@ skipping 121 matching lines @@
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   RtcEventLogHelperThread::ControlMessage message;
   message.message_type = RtcEventLogHelperThread::ControlMessage::START_FILE;
   message.max_size_bytes = max_size_bytes <= 0
                                ? std::numeric_limits<int64_t>::max()
                                : max_size_bytes;
   message.start_time = clock_->TimeInMicroseconds();
   message.stop_time = std::numeric_limits<int64_t>::max();
   message.file.reset(FileWrapper::Create());
   if (message.file->OpenFile(file_name.c_str(), false) != 0) {
+    LOG(LS_WARNING) << "Can't open file. Logging not started.";

[ivoc, 2016/05/26 08:56:58]

"Logging" sounds a bit vague to me, how about something like: RTC event log not
started.

[pbos-webrtc, 2016/05/26 12:05:18]

All of these are errors and should log on LS_ERROR.

[terelius, 2016/05/26 14:54:42]

Done.

[terelius, 2016/05/26 14:54:42]

Done.

     return false;
   }
   if (!message_queue_.Insert(&message)) {
     LOG(LS_WARNING) << "Message queue full. Can't start logging.";
     return false;
   }
   return true;
 }
 
 bool RtcEventLogImpl::StartLogging(rtc::PlatformFile platform_file,
                                    int64_t max_size_bytes) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   RtcEventLogHelperThread::ControlMessage message;
   message.message_type = RtcEventLogHelperThread::ControlMessage::START_FILE;
   message.max_size_bytes = max_size_bytes <= 0
                                ? std::numeric_limits<int64_t>::max()
                                : max_size_bytes;
   message.start_time = clock_->TimeInMicroseconds();
   message.stop_time = std::numeric_limits<int64_t>::max();
   message.file.reset(FileWrapper::Create());
   FILE* file_handle = rtc::FdopenPlatformFileForWriting(platform_file);
   if (!file_handle) {
+    LOG(LS_WARNING) << "Can't open file. Logging not started.";

[ivoc, 2016/05/26 08:56:59]

Same here.

[terelius, 2016/05/26 14:54:42]

Done.

+    // Even though we failed to open a FILE*, the platform_file is still open
+    // and needs to be closed.
+    if (!rtc::ClosePlatformFile(platform_file)) {
+      LOG(LS_WARNING) << "Can't close file.";
+    }
     return false;
   }
   if (message.file->OpenFromFileHandle(file_handle, true, false) != 0) {
+    LOG(LS_WARNING) << "Can't open file. Logging not started.";

[ivoc, 2016/05/26 08:56:59]

Same here.

[terelius, 2016/05/26 14:54:42]

Done.

     return false;
   }
   if (!message_queue_.Insert(&message)) {
     LOG(LS_WARNING) << "Message queue full. Can't start logging.";
     return false;
   }

[ivoc, 2016/05/26 08:56:58]

This is kind of unrelated to this CL, but I think it would be nice to have a
LOG(LS_INFO) message here to confirm that the logging has started successfully,
including the file name. WDYT?

[pbos-webrtc, 2016/05/26 12:05:18]

Agreed, that'd be useful.

[terelius, 2016/05/26 14:54:42]

Logging the filename would be very useful, but we don't actually know it here.
We could print a message "WebRTC event logging has started", but apart from
debugging the event log itself, I don't see the utility of this message. 

   return true;
 }
 
 void RtcEventLogImpl::StopLogging() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   RtcEventLogHelperThread::ControlMessage message;
   message.message_type = RtcEventLogHelperThread::ControlMessage::STOP_FILE;
   message.stop_time = clock_->TimeInMicroseconds();
   while (!message_queue_.Insert(&message)) {
     // TODO(terelius): We would like to have a blocking Insert function in the
@@ skipping 219 matching lines @@
 // RtcEventLog member functions.
 std::unique_ptr<RtcEventLog> RtcEventLog::Create(const Clock* clock) {
 #ifdef ENABLE_RTC_EVENT_LOG
   return std::unique_ptr<RtcEventLog>(new RtcEventLogImpl(clock));
 #else
   return std::unique_ptr<RtcEventLog>(new RtcEventLogNullImpl());
 #endif  // ENABLE_RTC_EVENT_LOG
 }
 
 }  // namespace webrtc