From 0d81443b766dc1a45b44054e0fee03bcc0854b79 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Wed, 23 Oct 2019 11:32:26 -0700 Subject: [PATCH] Fix memory leaks in mediaanalytics This fixes > a dozen potential leaks flagged by clang's static analyzer, and fixes the use of other nearby manual memory management. Bug: None Test: TreeHugger Change-Id: I8af2523804986b278150291fb2dc7ca5af8391ee --- .../mediaanalytics/statsd_audiopolicy.cpp | 44 ++++++-------- .../mediaanalytics/statsd_audiorecord.cpp | 34 +++++------ .../mediaanalytics/statsd_audiothread.cpp | 58 ++++++++----------- services/mediaanalytics/statsd_audiotrack.cpp | 37 +++++------- services/mediaanalytics/statsd_codec.cpp | 30 ++++------ services/mediaanalytics/statsd_extractor.cpp | 16 ++--- services/mediaanalytics/statsd_nuplayer.cpp | 44 ++++++-------- services/mediaanalytics/statsd_recorder.cpp | 16 ++--- 8 files changed, 113 insertions(+), 166 deletions(-) diff --git a/services/mediaanalytics/statsd_audiopolicy.cpp b/services/mediaanalytics/statsd_audiopolicy.cpp index 06c4ddebcf..95cb274167 100644 --- a/services/mediaanalytics/statsd_audiopolicy.cpp +++ b/services/mediaanalytics/statsd_audiopolicy.cpp @@ -60,14 +60,14 @@ bool statsd_audiopolicy(MediaAnalyticsItem *item) metrics_proto.set_status(status); } //string char kAudioPolicyRqstSrc[] = "android.media.audiopolicy.rqst.src"; - char *rqst_src = NULL; - if (item->getCString("android.media.audiopolicy.rqst.src", &rqst_src)) { - metrics_proto.set_request_source(rqst_src); + std::string rqst_src; + if (item->getString("android.media.audiopolicy.rqst.src", &rqst_src)) { + metrics_proto.set_request_source(std::move(rqst_src)); } //string char kAudioPolicyRqstPkg[] = "android.media.audiopolicy.rqst.pkg"; - char *rqst_pkg = NULL; - if (item->getCString("android.media.audiopolicy.rqst.pkg", &rqst_pkg)) { - metrics_proto.set_request_package(rqst_pkg); + std::string rqst_pkg; + if (item->getString("android.media.audiopolicy.rqst.pkg", &rqst_pkg)) { + metrics_proto.set_request_package(std::move(rqst_pkg)); } //int32 char kAudioPolicyRqstSession[] = "android.media.audiopolicy.rqst.session"; int32_t rqst_session = -1; @@ -75,20 +75,20 @@ bool statsd_audiopolicy(MediaAnalyticsItem *item) metrics_proto.set_request_session(rqst_session); } //string char kAudioPolicyRqstDevice[] = "android.media.audiopolicy.rqst.device"; - char *rqst_device = NULL; - if (item->getCString("android.media.audiopolicy.rqst.device", &rqst_device)) { - metrics_proto.set_request_device(rqst_device); + std::string rqst_device; + if (item->getString("android.media.audiopolicy.rqst.device", &rqst_device)) { + metrics_proto.set_request_device(std::move(rqst_device)); } //string char kAudioPolicyActiveSrc[] = "android.media.audiopolicy.active.src"; - char *active_src = NULL; - if (item->getCString("android.media.audiopolicy.active.src", &active_src)) { - metrics_proto.set_active_source(active_src); + std::string active_src; + if (item->getString("android.media.audiopolicy.active.src", &active_src)) { + metrics_proto.set_active_source(std::move(active_src)); } //string char kAudioPolicyActivePkg[] = "android.media.audiopolicy.active.pkg"; - char *active_pkg = NULL; - if (item->getCString("android.media.audiopolicy.active.pkg", &active_pkg)) { - metrics_proto.set_active_package(active_pkg); + std::string active_pkg; + if (item->getString("android.media.audiopolicy.active.pkg", &active_pkg)) { + metrics_proto.set_active_package(std::move(active_pkg)); } //int32 char kAudioPolicyActiveSession[] = "android.media.audiopolicy.active.session"; int32_t active_session = -1; @@ -96,9 +96,9 @@ bool statsd_audiopolicy(MediaAnalyticsItem *item) metrics_proto.set_active_session(active_session); } //string char kAudioPolicyActiveDevice[] = "android.media.audiopolicy.active.device"; - char *active_device = NULL; - if (item->getCString("android.media.audiopolicy.active.device", &active_device)) { - metrics_proto.set_active_device(active_device); + std::string active_device; + if (item->getString("android.media.audiopolicy.active.device", &active_device)) { + metrics_proto.set_active_device(std::move(active_device)); } @@ -119,14 +119,6 @@ bool statsd_audiopolicy(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(rqst_src); - free(rqst_pkg); - free(rqst_device); - free(active_src); - free(active_pkg); - free(active_device); - return true; } diff --git a/services/mediaanalytics/statsd_audiorecord.cpp b/services/mediaanalytics/statsd_audiorecord.cpp index c9edb27f4a..7c7a62c67e 100644 --- a/services/mediaanalytics/statsd_audiorecord.cpp +++ b/services/mediaanalytics/statsd_audiorecord.cpp @@ -54,14 +54,14 @@ bool statsd_audiorecord(MediaAnalyticsItem *item) // flesh out the protobuf we'll hand off with our data // - char *encoding = NULL; - if (item->getCString("android.media.audiorecord.encoding", &encoding)) { - metrics_proto.set_encoding(encoding); + std::string encoding; + if (item->getString("android.media.audiorecord.encoding", &encoding)) { + metrics_proto.set_encoding(std::move(encoding)); } - char *source = NULL; - if (item->getCString("android.media.audiorecord.source", &source)) { - metrics_proto.set_source(source); + std::string source; + if (item->getString("android.media.audiorecord.source", &source)) { + metrics_proto.set_source(std::move(source)); } int32_t latency = -1; @@ -101,11 +101,11 @@ bool statsd_audiorecord(MediaAnalyticsItem *item) metrics_proto.set_error_code(errcode); } - char *errfunc = NULL; - if (item->getCString("android.media.audiorecord.errfunc", &errfunc)) { - metrics_proto.set_error_function(errfunc); - } else if (item->getCString("android.media.audiorecord.lastError.at", &errfunc)) { - metrics_proto.set_error_function(errfunc); + std::string errfunc; + if (item->getString("android.media.audiorecord.errfunc", &errfunc)) { + metrics_proto.set_error_function(std::move(errfunc)); + } else if (item->getString("android.media.audiorecord.lastError.at", &errfunc)) { + metrics_proto.set_error_function(std::move(errfunc)); } // portId (int32) @@ -119,9 +119,9 @@ bool statsd_audiorecord(MediaAnalyticsItem *item) metrics_proto.set_frame_count(frameCount); } // attributes (string) - char *attributes = NULL; - if (item->getCString("android.media.audiorecord.attributes", &attributes)) { - metrics_proto.set_attributes(attributes); + std::string attributes; + if (item->getString("android.media.audiorecord.attributes", &attributes)) { + metrics_proto.set_attributes(std::move(attributes)); } // channelMask (int64) int64_t channelMask = -1; @@ -152,12 +152,6 @@ bool statsd_audiorecord(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(encoding); - free(source); - free(errfunc); - free(attributes); - return true; } diff --git a/services/mediaanalytics/statsd_audiothread.cpp b/services/mediaanalytics/statsd_audiothread.cpp index 8232424839..e9d6b17866 100644 --- a/services/mediaanalytics/statsd_audiothread.cpp +++ b/services/mediaanalytics/statsd_audiothread.cpp @@ -56,9 +56,9 @@ bool statsd_audiothread(MediaAnalyticsItem *item) // flesh out the protobuf we'll hand off with our data // - char *mytype = NULL; - if (item->getCString(MM_PREFIX "type", &mytype)) { - metrics_proto.set_type(mytype); + std::string mytype; + if (item->getString(MM_PREFIX "type", &mytype)) { + metrics_proto.set_type(std::move(mytype)); } int32_t framecount = -1; if (item->getInt32(MM_PREFIX "framecount", &framecount)) { @@ -68,17 +68,17 @@ bool statsd_audiothread(MediaAnalyticsItem *item) if (item->getInt32(MM_PREFIX "samplerate", &samplerate)) { metrics_proto.set_samplerate(samplerate); } - char *workhist = NULL; - if (item->getCString(MM_PREFIX "workMs.hist", &workhist)) { - metrics_proto.set_work_millis_hist(workhist); + std::string workhist; + if (item->getString(MM_PREFIX "workMs.hist", &workhist)) { + metrics_proto.set_work_millis_hist(std::move(workhist)); } - char *latencyhist = NULL; - if (item->getCString(MM_PREFIX "latencyMs.hist", &latencyhist)) { - metrics_proto.set_latency_millis_hist(latencyhist); + std::string latencyhist; + if (item->getString(MM_PREFIX "latencyMs.hist", &latencyhist)) { + metrics_proto.set_latency_millis_hist(std::move(latencyhist)); } - char *warmuphist = NULL; - if (item->getCString(MM_PREFIX "warmupMs.hist", &warmuphist)) { - metrics_proto.set_warmup_millis_hist(warmuphist); + std::string warmuphist; + if (item->getString(MM_PREFIX "warmupMs.hist", &warmuphist)) { + metrics_proto.set_warmup_millis_hist(std::move(warmuphist)); } int64_t underruns = -1; if (item->getInt64(MM_PREFIX "underruns", &underruns)) { @@ -108,9 +108,9 @@ bool statsd_audiothread(MediaAnalyticsItem *item) metrics_proto.set_port_id(port_id); } // item->setCString(MM_PREFIX "type", threadTypeToString(mType)); - char *type = NULL; - if (item->getCString(MM_PREFIX "type", &type)) { - metrics_proto.set_type(type); + std::string type; + if (item->getString(MM_PREFIX "type", &type)) { + metrics_proto.set_type(std::move(type)); } // item->setInt32(MM_PREFIX "sampleRate", (int32_t)mSampleRate); int32_t sample_rate = -1; @@ -123,9 +123,9 @@ bool statsd_audiothread(MediaAnalyticsItem *item) metrics_proto.set_channel_mask(channel_mask); } // item->setCString(MM_PREFIX "encoding", toString(mFormat).c_str()); - char *encoding = NULL; - if (item->getCString(MM_PREFIX "encoding", &encoding)) { - metrics_proto.set_encoding(encoding); + std::string encoding; + if (item->getString(MM_PREFIX "encoding", &encoding)) { + metrics_proto.set_encoding(std::move(encoding)); } // item->setInt32(MM_PREFIX "frameCount", (int32_t)mFrameCount); int32_t frame_count = -1; @@ -133,14 +133,14 @@ bool statsd_audiothread(MediaAnalyticsItem *item) metrics_proto.set_frame_count(frame_count); } // item->setCString(MM_PREFIX "outDevice", toString(mOutDevice).c_str()); - char *outDevice = NULL; - if (item->getCString(MM_PREFIX "outDevice", &outDevice)) { - metrics_proto.set_output_device(outDevice); + std::string outDevice; + if (item->getString(MM_PREFIX "outDevice", &outDevice)) { + metrics_proto.set_output_device(std::move(outDevice)); } // item->setCString(MM_PREFIX "inDevice", toString(mInDevice).c_str()); - char *inDevice = NULL; - if (item->getCString(MM_PREFIX "inDevice", &inDevice)) { - metrics_proto.set_input_device(inDevice); + std::string inDevice; + if (item->getString(MM_PREFIX "inDevice", &inDevice)) { + metrics_proto.set_input_device(std::move(inDevice)); } // item->setDouble(MM_PREFIX "ioJitterMs.mean", mIoJitterMs.getMean()); double iojitters_ms_mean = -1; @@ -201,16 +201,6 @@ bool statsd_audiothread(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(mytype); - free(workhist); - free(latencyhist); - free(warmuphist); - free(type); - free(encoding); - free(inDevice); - free(outDevice); - return true; } diff --git a/services/mediaanalytics/statsd_audiotrack.cpp b/services/mediaanalytics/statsd_audiotrack.cpp index f250ced8d1..57cda99495 100644 --- a/services/mediaanalytics/statsd_audiotrack.cpp +++ b/services/mediaanalytics/statsd_audiotrack.cpp @@ -57,23 +57,23 @@ bool statsd_audiotrack(MediaAnalyticsItem *item) // static constexpr char kAudioTrackStreamType[] = "android.media.audiotrack.streamtype"; // optional string streamType; - char *streamtype = NULL; - if (item->getCString("android.media.audiotrack.streamtype", &streamtype)) { - metrics_proto.set_stream_type(streamtype); + std::string streamtype; + if (item->getString("android.media.audiotrack.streamtype", &streamtype)) { + metrics_proto.set_stream_type(std::move(streamtype)); } // static constexpr char kAudioTrackContentType[] = "android.media.audiotrack.type"; // optional string contentType; - char *contenttype = NULL; - if (item->getCString("android.media.audiotrack.type", &contenttype)) { - metrics_proto.set_content_type(contenttype); + std::string contenttype; + if (item->getString("android.media.audiotrack.type", &contenttype)) { + metrics_proto.set_content_type(std::move(contenttype)); } // static constexpr char kAudioTrackUsage[] = "android.media.audiotrack.usage"; // optional string trackUsage; - char *trackusage = NULL; - if (item->getCString("android.media.audiotrack.usage", &trackusage)) { - metrics_proto.set_track_usage(trackusage); + std::string trackusage; + if (item->getString("android.media.audiotrack.usage", &trackusage)) { + metrics_proto.set_track_usage(std::move(trackusage)); } // static constexpr char kAudioTrackSampleRate[] = "android.media.audiotrack.samplerate"; @@ -111,9 +111,9 @@ bool statsd_audiotrack(MediaAnalyticsItem *item) metrics_proto.set_port_id(port_id); } // encoding (string) - char *encoding = NULL; - if (item->getCString("android.media.audiotrack.encoding", &encoding)) { - metrics_proto.set_encoding(encoding); + std::string encoding; + if (item->getString("android.media.audiotrack.encoding", &encoding)) { + metrics_proto.set_encoding(std::move(encoding)); } // frameCount (int32) int32_t frame_count = -1; @@ -121,9 +121,9 @@ bool statsd_audiotrack(MediaAnalyticsItem *item) metrics_proto.set_frame_count(frame_count); } // attributes (string) - char *attributes = NULL; - if (item->getCString("android.media.audiotrack.attributes", &attributes)) { - metrics_proto.set_attributes(attributes); + std::string attributes; + if (item->getString("android.media.audiotrack.attributes", &attributes)) { + metrics_proto.set_attributes(std::move(attributes)); } std::string serialized; @@ -143,13 +143,6 @@ bool statsd_audiotrack(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(streamtype); - free(contenttype); - free(trackusage); - free(encoding); - free(attributes); - return true; } diff --git a/services/mediaanalytics/statsd_codec.cpp b/services/mediaanalytics/statsd_codec.cpp index dc8e4ef18c..bf82e50d07 100644 --- a/services/mediaanalytics/statsd_codec.cpp +++ b/services/mediaanalytics/statsd_codec.cpp @@ -55,19 +55,19 @@ bool statsd_codec(MediaAnalyticsItem *item) // flesh out the protobuf we'll hand off with our data // // android.media.mediacodec.codec string - char *codec = NULL; - if (item->getCString("android.media.mediacodec.codec", &codec)) { - metrics_proto.set_codec(codec); + std::string codec; + if (item->getString("android.media.mediacodec.codec", &codec)) { + metrics_proto.set_codec(std::move(codec)); } // android.media.mediacodec.mime string - char *mime = NULL; - if (item->getCString("android.media.mediacodec.mime", &mime)) { - metrics_proto.set_mime(mime); + std::string mime; + if (item->getString("android.media.mediacodec.mime", &mime)) { + metrics_proto.set_mime(std::move(mime)); } // android.media.mediacodec.mode string - char *mode = NULL; - if ( item->getCString("android.media.mediacodec.mode", &mode)) { - metrics_proto.set_mode(mode); + std::string mode; + if ( item->getString("android.media.mediacodec.mode", &mode)) { + metrics_proto.set_mode(std::move(mode)); } // android.media.mediacodec.encoder int32 int32_t encoder = -1; @@ -125,9 +125,9 @@ bool statsd_codec(MediaAnalyticsItem *item) metrics_proto.set_error_code(errcode); } // android.media.mediacodec.errstate string - char *errstate = NULL; - if ( item->getCString("android.media.mediacodec.errstate", &errstate)) { - metrics_proto.set_error_state(errstate); + std::string errstate; + if ( item->getString("android.media.mediacodec.errstate", &errstate)) { + metrics_proto.set_error_state(std::move(errstate)); } // android.media.mediacodec.latency.max int64 int64_t latency_max = -1; @@ -173,12 +173,6 @@ bool statsd_codec(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(codec); - free(mime); - free(mode); - free(errstate); - return true; } diff --git a/services/mediaanalytics/statsd_extractor.cpp b/services/mediaanalytics/statsd_extractor.cpp index 395c91202d..d84930cb3a 100644 --- a/services/mediaanalytics/statsd_extractor.cpp +++ b/services/mediaanalytics/statsd_extractor.cpp @@ -56,14 +56,14 @@ bool statsd_extractor(MediaAnalyticsItem *item) // // android.media.mediaextractor.fmt string - char *fmt = NULL; - if (item->getCString("android.media.mediaextractor.fmt", &fmt)) { - metrics_proto.set_format(fmt); + std::string fmt; + if (item->getString("android.media.mediaextractor.fmt", &fmt)) { + metrics_proto.set_format(std::move(fmt)); } // android.media.mediaextractor.mime string - char *mime = NULL; - if (item->getCString("android.media.mediaextractor.mime", &mime)) { - metrics_proto.set_mime(mime); + std::string mime; + if (item->getString("android.media.mediaextractor.mime", &mime)) { + metrics_proto.set_mime(std::move(mime)); } // android.media.mediaextractor.ntrk int32 int32_t ntrk = -1; @@ -88,10 +88,6 @@ bool statsd_extractor(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(fmt); - free(mime); - return true; } diff --git a/services/mediaanalytics/statsd_nuplayer.cpp b/services/mediaanalytics/statsd_nuplayer.cpp index 5ec118a6dc..e6e0f2cf5a 100644 --- a/services/mediaanalytics/statsd_nuplayer.cpp +++ b/services/mediaanalytics/statsd_nuplayer.cpp @@ -62,13 +62,13 @@ bool statsd_nuplayer(MediaAnalyticsItem *item) // differentiate between nuplayer and nuplayer2 metrics_proto.set_whichplayer(item->getKey().c_str()); - char *video_mime = NULL; - if (item->getCString("android.media.mediaplayer.video.mime", &video_mime)) { - metrics_proto.set_video_mime(video_mime); + std::string video_mime; + if (item->getString("android.media.mediaplayer.video.mime", &video_mime)) { + metrics_proto.set_video_mime(std::move(video_mime)); } - char *video_codec = NULL; - if (item->getCString("android.media.mediaplayer.video.codec", &video_codec)) { - metrics_proto.set_video_codec(video_codec); + std::string video_codec; + if (item->getString("android.media.mediaplayer.video.codec", &video_codec)) { + metrics_proto.set_video_codec(std::move(video_codec)); } int32_t width = -1; @@ -97,13 +97,13 @@ bool statsd_nuplayer(MediaAnalyticsItem *item) metrics_proto.set_framerate(fps); } - char *audio_mime = NULL; - if (item->getCString("android.media.mediaplayer.audio.mime", &audio_mime)) { - metrics_proto.set_audio_mime(audio_mime); + std::string audio_mime; + if (item->getString("android.media.mediaplayer.audio.mime", &audio_mime)) { + metrics_proto.set_audio_mime(std::move(audio_mime)); } - char *audio_codec = NULL; - if (item->getCString("android.media.mediaplayer.audio.codec", &audio_codec)) { - metrics_proto.set_audio_codec(audio_codec); + std::string audio_codec; + if (item->getString("android.media.mediaplayer.audio.codec", &audio_codec)) { + metrics_proto.set_audio_codec(std::move(audio_codec)); } int64_t duration_ms = -1; @@ -123,14 +123,14 @@ bool statsd_nuplayer(MediaAnalyticsItem *item) if (item->getInt32("android.media.mediaplayer.errcode", &error_code)) { metrics_proto.set_error_code(error_code); } - char *error_state = NULL; - if (item->getCString("android.media.mediaplayer.errstate", &error_state)) { - metrics_proto.set_error_state(error_state); + std::string error_state; + if (item->getString("android.media.mediaplayer.errstate", &error_state)) { + metrics_proto.set_error_state(std::move(error_state)); } - char *data_source_type = NULL; - if (item->getCString("android.media.mediaplayer.dataSource", &data_source_type)) { - metrics_proto.set_data_source_type(data_source_type); + std::string data_source_type; + if (item->getString("android.media.mediaplayer.dataSource", &data_source_type)) { + metrics_proto.set_data_source_type(std::move(data_source_type)); } int64_t rebufferingMs = -1; @@ -164,14 +164,6 @@ bool statsd_nuplayer(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(video_mime); - free(video_codec); - free(audio_mime); - free(audio_codec); - free(error_state); - free(data_source_type); - return true; } diff --git a/services/mediaanalytics/statsd_recorder.cpp b/services/mediaanalytics/statsd_recorder.cpp index 4d981b44cc..d286f001c7 100644 --- a/services/mediaanalytics/statsd_recorder.cpp +++ b/services/mediaanalytics/statsd_recorder.cpp @@ -56,14 +56,14 @@ bool statsd_recorder(MediaAnalyticsItem *item) // // string kRecorderAudioMime = "android.media.mediarecorder.audio.mime"; - char *audio_mime = NULL; - if (item->getCString("android.media.mediarecorder.audio.mime", &audio_mime)) { - metrics_proto.set_audio_mime(audio_mime); + std::string audio_mime; + if (item->getString("android.media.mediarecorder.audio.mime", &audio_mime)) { + metrics_proto.set_audio_mime(std::move(audio_mime)); } // string kRecorderVideoMime = "android.media.mediarecorder.video.mime"; - char *video_mime = NULL; - if (item->getCString("android.media.mediarecorder.video.mime", &video_mime)) { - metrics_proto.set_video_mime(video_mime); + std::string video_mime; + if (item->getString("android.media.mediarecorder.video.mime", &video_mime)) { + metrics_proto.set_video_mime(std::move(video_mime)); } // int32 kRecorderVideoProfile = "android.media.mediarecorder.video-encoder-profile"; int32_t videoProfile = -1; @@ -183,10 +183,6 @@ bool statsd_recorder(MediaAnalyticsItem *item) ALOGV("NOT sending: private data (len=%zu)", strlen(serialized.c_str())); } - // must free the strings that we were given - free(audio_mime); - free(video_mime); - return true; }