From 87973bf516432756f6948e969dbe9d6aa8a27017 Mon Sep 17 00:00:00 2001 From: Jiri Novak Date: Thu, 21 Jun 2018 15:16:03 +0200 Subject: [PATCH] RTP: Common functions for allocation/deallocation of rtpstream_info_t Change-Id: I9a0a11d238473a7c57d85547dca0713ed421a500 Reviewed-on: https://code.wireshark.org/review/28417 Petri-Dish: Alexis La Goutte Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- tshark.c | 2 + ui/qt/iax2_analysis_dialog.cpp | 16 +++++-- ui/qt/rtp_analysis_dialog.cpp | 14 +++--- ui/qt/rtp_audio_stream.cpp | 6 ++- ui/qt/rtp_audio_stream.h | 5 +- ui/qt/rtp_player_dialog.cpp | 43 ++++++++--------- ui/qt/rtp_player_dialog.h | 6 +-- ui/qt/rtp_stream_dialog.cpp | 9 +++- ui/tap-rtp-common.c | 86 +++++++++++++++++++++++++++++----- ui/tap-rtp-common.h | 10 ++++ ui/voip_calls.c | 13 +++-- 11 files changed, 153 insertions(+), 57 deletions(-) diff --git a/tshark.c b/tshark.c index 64dadac220..3f20f0129a 100644 --- a/tshark.c +++ b/tshark.c @@ -2229,6 +2229,8 @@ main(int argc, char *argv[]) } draw_tap_listeners(TRUE); + /* Memory cleanup */ + reset_tap_listeners(); funnel_dump_all_text_windows(); epan_free(cfile.epan); epan_cleanup(); diff --git a/ui/qt/iax2_analysis_dialog.cpp b/ui/qt/iax2_analysis_dialog.cpp index 7bb3bd5cf7..996f1b2ee1 100644 --- a/ui/qt/iax2_analysis_dialog.cpp +++ b/ui/qt/iax2_analysis_dialog.cpp @@ -342,7 +342,8 @@ Iax2AnalysisDialog::Iax2AnalysisDialog(QWidget &parent, CaptureFile &cf) : rtpstream_tapinfo_t tapinfo; /* Register the tap listener */ - memset(&tapinfo, 0, sizeof(rtpstream_tapinfo_t)); + rtpstream_info_init(&tapinfo); + tapinfo.tap_data = this; tapinfo.mode = TAP_ANALYSE; @@ -351,21 +352,28 @@ Iax2AnalysisDialog::Iax2AnalysisDialog(QWidget &parent, CaptureFile &cf) : rtpstream_scan(&tapinfo, cap_file_.capFile(), NULL); int num_streams = 0; - GList *filtered_list = NULL; + // TODO: Not used + //GList *filtered_list = NULL; for (GList *strinfo_list = g_list_first(tapinfo.strinfo_list); strinfo_list; strinfo_list = g_list_next(strinfo_list)) { rtpstream_info_t * strinfo = (rtpstream_info_t*)(strinfo_list->data); if (rtpstream_id_equal(&(strinfo->id), &(fwd_id_),RTPSTREAM_ID_EQUAL_NONE)) { ++num_streams; - filtered_list = g_list_prepend(filtered_list, strinfo); + // TODO: Not used + //filtered_list = g_list_prepend(filtered_list, strinfo); } if (rtpstream_id_equal(&(strinfo->id), &(rev_id_),RTPSTREAM_ID_EQUAL_NONE)) { ++num_streams; - filtered_list = g_list_append(filtered_list, strinfo); + // TODO: Not used + //filtered_list = g_list_append(filtered_list, strinfo); } + + rtpstream_info_free_data(strinfo); + g_free(list->data); } + g_list_free(tapinfo->strinfo_list); if (num_streams > 1) { // Open the RTP streams dialog. diff --git a/ui/qt/rtp_analysis_dialog.cpp b/ui/qt/rtp_analysis_dialog.cpp index 3f3340b0c4..9269c0e3a9 100644 --- a/ui/qt/rtp_analysis_dialog.cpp +++ b/ui/qt/rtp_analysis_dialog.cpp @@ -346,10 +346,10 @@ RtpAnalysisDialog::RtpAnalysisDialog(QWidget &parent, CaptureFile &cf, rtpstream ui->buttonBox->button(QDialogButtonBox::Save)->setMenu(save_menu); if (stream_fwd) { // XXX What if stream_fwd == 0 && stream_rev != 0? - fwd_statinfo_ = *stream_fwd; + rtpstream_info_copy_deep(&fwd_statinfo_, stream_fwd); num_streams_=1; if (stream_rev) { - rev_statinfo_ = *stream_rev; + rtpstream_info_copy_deep(&rev_statinfo_, stream_rev); num_streams_=2; } } else { @@ -381,8 +381,8 @@ RtpAnalysisDialog::~RtpAnalysisDialog() { delete ui; // remove_tap_listener_rtpstream(&tapinfo_); - rtpstream_id_free(&(fwd_statinfo_.id)); - rtpstream_id_free(&(rev_statinfo_.id)); + rtpstream_info_free_data(&fwd_statinfo_); + rtpstream_info_free_data(&rev_statinfo_); delete fwd_tempfile_; delete rev_tempfile_; } @@ -1007,15 +1007,15 @@ void RtpAnalysisDialog::showPlayer() // XXX We might want to create an "rtp_stream_id_t" struct with only // addresses, ports & SSRC. - memset(&stream_info, 0, sizeof(stream_info)); + rtpstream_info_init(&stream_info); rtpstream_id_copy(&fwd_statinfo_.id, &stream_info.id); stream_info.packet_count = fwd_statinfo_.packet_count; stream_info.setup_frame_number = fwd_statinfo_.setup_frame_number; nstime_copy(&stream_info.start_rel_time, &fwd_statinfo_.start_rel_time); - rtp_player_dialog.addRtpStream(&stream_info); + if (num_streams_ > 1) { - memset(&stream_info, 0, sizeof(stream_info)); + rtpstream_info_init(&stream_info); rtpstream_id_copy(&rev_statinfo_.id, &stream_info.id); stream_info.packet_count = rev_statinfo_.packet_count; stream_info.setup_frame_number = rev_statinfo_.setup_frame_number; diff --git a/ui/qt/rtp_audio_stream.cpp b/ui/qt/rtp_audio_stream.cpp index 5b422cc2da..3f05cfd9cc 100644 --- a/ui/qt/rtp_audio_stream.cpp +++ b/ui/qt/rtp_audio_stream.cpp @@ -100,13 +100,17 @@ bool RtpAudioStream::isMatch(const _packet_info *pinfo, const _rtp_info *rtp_inf // XXX We add multiple RTP streams here because that's what the GTK+ UI does. // Should we make these distinct, with their own waveforms? It seems like // that would simplify a lot of things. +// TODO: It is not used +/* void RtpAudioStream::addRtpStream(const rtpstream_info_t *rtpstream) { if (!rtpstream) return; // RTP_STREAM_DEBUG("added %d:%u packets", g_list_length(rtpstream->rtp_packet_list), rtpstream->packet_count); - rtpstreams_ << rtpstream; + // TODO: It is not used + //rtpstreams_ << rtpstream; } +*/ void RtpAudioStream::addRtpPacket(const struct _packet_info *pinfo, const struct _rtp_info *rtp_info) { diff --git a/ui/qt/rtp_audio_stream.h b/ui/qt/rtp_audio_stream.h index cc69dc1625..33d56fd5db 100644 --- a/ui/qt/rtp_audio_stream.h +++ b/ui/qt/rtp_audio_stream.h @@ -43,7 +43,7 @@ public: ~RtpAudioStream(); bool isMatch(const rtpstream_info_t *rtpstream) const; bool isMatch(const struct _packet_info *pinfo, const struct _rtp_info *rtp_info) const; - void addRtpStream(const rtpstream_info_t *rtpstream); + //void addRtpStream(const rtpstream_info_t *rtpstream); void addRtpPacket(const struct _packet_info *pinfo, const struct _rtp_info *rtp_info); void reset(double start_rel_time); void decode(); @@ -146,7 +146,8 @@ private: QVectorrtp_packets_; QTemporaryFile *tempfile_; struct _GHashTable *decoders_hash_; - QListrtpstreams_; + // TODO: It is not used + //QListrtpstreams_; double global_start_rel_time_; double start_abs_offset_; double start_rel_time_; diff --git a/ui/qt/rtp_player_dialog.cpp b/ui/qt/rtp_player_dialog.cpp index 2f1d9a37ce..091b4267ea 100644 --- a/ui/qt/rtp_player_dialog.cpp +++ b/ui/qt/rtp_player_dialog.cpp @@ -353,9 +353,9 @@ void RtpPlayerDialog::rescanPackets(bool rescale_axes) updateWidgets(); } -void RtpPlayerDialog::addRtpStream(rtpstream_info_t *rtp_stream) +void RtpPlayerDialog::addRtpStream(rtpstream_info_t *rtpstream) { - if (!rtp_stream) return; + if (!rtpstream) return; // Find the RTP streams associated with this conversation. // gtk/rtp_player.c:mark_rtp_stream_to_play does this differently. @@ -365,24 +365,24 @@ void RtpPlayerDialog::addRtpStream(rtpstream_info_t *rtp_stream) for (int row = 0; row < tli_count; row++) { QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row); RtpAudioStream *row_stream = ti->data(stream_data_col_, Qt::UserRole).value(); - if (row_stream->isMatch(rtp_stream)) { + if (row_stream->isMatch(rtpstream)) { audio_stream = row_stream; break; } } if (!audio_stream) { - audio_stream = new RtpAudioStream(this, rtp_stream); + audio_stream = new RtpAudioStream(this, rtpstream); audio_stream->setColor(ColorUtils::graphColor(tli_count)); QTreeWidgetItem *ti = new QTreeWidgetItem(ui->streamTreeWidget); - ti->setText(src_addr_col_, address_to_qstring(&rtp_stream->id.src_addr)); - ti->setText(src_port_col_, QString::number(rtp_stream->id.src_port)); - ti->setText(dst_addr_col_, address_to_qstring(&rtp_stream->id.dst_addr)); - ti->setText(dst_port_col_, QString::number(rtp_stream->id.dst_port)); - ti->setText(ssrc_col_, int_to_qstring(rtp_stream->id.ssrc, 8, 16)); - ti->setText(first_pkt_col_, QString::number(rtp_stream->setup_frame_number)); - ti->setText(num_pkts_col_, QString::number(rtp_stream->packet_count)); + ti->setText(src_addr_col_, address_to_qstring(&rtpstream->id.src_addr)); + ti->setText(src_port_col_, QString::number(rtpstream->id.src_port)); + ti->setText(dst_addr_col_, address_to_qstring(&rtpstream->id.dst_addr)); + ti->setText(dst_port_col_, QString::number(rtpstream->id.dst_port)); + ti->setText(ssrc_col_, int_to_qstring(rtpstream->id.ssrc, 8, 16)); + ti->setText(first_pkt_col_, QString::number(rtpstream->setup_frame_number)); + ti->setText(num_pkts_col_, QString::number(rtpstream->packet_count)); ti->setData(stream_data_col_, Qt::UserRole, QVariant::fromValue(audio_stream)); @@ -398,8 +398,9 @@ void RtpPlayerDialog::addRtpStream(rtpstream_info_t *rtp_stream) connect(audio_stream, SIGNAL(playbackError(QString)), this, SLOT(setPlaybackError(QString))); connect(audio_stream, SIGNAL(processedSecs(double)), this, SLOT(setPlayPosition(double))); } - audio_stream->addRtpStream(rtp_stream); - double start_rel_time = nstime_to_sec(&rtp_stream->start_rel_time); + // TODO: does not do anything + // audio_stream->addRtpStream(rtpstream); + double start_rel_time = nstime_to_sec(&rtpstream->start_rel_time); if (tli_count < 2) { start_rel_time_ = start_rel_time; } else { @@ -407,8 +408,8 @@ void RtpPlayerDialog::addRtpStream(rtpstream_info_t *rtp_stream) } RTP_STREAM_DEBUG("adding stream %d to layout, %u packets, start %u", ui->streamTreeWidget->topLevelItemCount(), - rtp_stream->packet_count, - rtp_stream->start_fd ? rtp_stream->start_fd->num : 0); + rtpstream->packet_count, + rtpstream->start_fd ? rtpstream->start_fd->num : 0); } void RtpPlayerDialog::showEvent(QShowEvent *) @@ -770,14 +771,14 @@ void RtpPlayerDialog::on_buttonBox_helpRequested() #if 0 // This also serves as a title in RtpAudioFrame. static const QString stream_key_tmpl_ = "%1:%2 " UTF8_RIGHTWARDS_ARROW " %3:%4 0x%5"; -const QString RtpPlayerDialog::streamKey(const rtpstream_info_t *rtp_stream) +const QString RtpPlayerDialog::streamKey(const rtpstream_info_t *rtpstream) { const QString stream_key = QString(stream_key_tmpl_) - .arg(address_to_display_qstring(&rtp_stream->src_addr)) - .arg(rtp_stream->src_port) - .arg(address_to_display_qstring(&rtp_stream->dst_addr)) - .arg(rtp_stream->dst_port) - .arg(rtp_stream->ssrc, 0, 16); + .arg(address_to_display_qstring(&rtpstream->src_addr)) + .arg(rtpstream->src_port) + .arg(address_to_display_qstring(&rtpstream->dst_addr)) + .arg(rtpstream->dst_port) + .arg(rtpstream->ssrc, 0, 16); return stream_key; } diff --git a/ui/qt/rtp_player_dialog.h b/ui/qt/rtp_player_dialog.h index da8b1fb218..d9e2581059 100644 --- a/ui/qt/rtp_player_dialog.h +++ b/ui/qt/rtp_player_dialog.h @@ -58,9 +58,9 @@ public: * Requires src_addr, src_port, dest_addr, dest_port, ssrc, packet_count, * setup_frame_number, and start_rel_time. * - * @param rtp_stream struct with rtp_stream info + * @param rtpstream struct with rtpstream info */ - void addRtpStream(rtpstream_info_t *rtp_stream); + void addRtpStream(rtpstream_info_t *rtpstream); public slots: @@ -113,7 +113,7 @@ private: QCPItemStraightLine *cur_play_pos_; QString playback_error_; -// const QString streamKey(const rtpstream_info_t *rtp_stream); +// const QString streamKey(const rtpstream_info_t *rtpstream); // const QString streamKey(const packet_info *pinfo, const struct _rtp_info *rtpinfo); // Tap callbacks diff --git a/ui/qt/rtp_stream_dialog.cpp b/ui/qt/rtp_stream_dialog.cpp index 7b4ba59715..8082231937 100644 --- a/ui/qt/rtp_stream_dialog.cpp +++ b/ui/qt/rtp_stream_dialog.cpp @@ -74,12 +74,17 @@ class RtpStreamTreeWidgetItem : public QTreeWidgetItem { public: RtpStreamTreeWidgetItem(QTreeWidget *tree, rtpstream_info_t *stream_info) : - QTreeWidgetItem(tree, rtp_stream_type_), - stream_info_(stream_info) + QTreeWidgetItem(tree, rtp_stream_type_) { + stream_info_=rtpstream_info_malloc_and_copy_deep(stream_info); drawData(); } + ~RtpStreamTreeWidgetItem() + { + rtpstream_info_free_all(stream_info_); + } + rtpstream_info_t *streamInfo() const { return stream_info_; } void drawData() { diff --git a/ui/tap-rtp-common.c b/ui/tap-rtp-common.c index fc3b006c84..267172b53a 100644 --- a/ui/tap-rtp-common.c +++ b/ui/tap-rtp-common.c @@ -44,12 +44,76 @@ typedef struct st_rtpdump_info { const guint8 *samples; /**< data bytes */ } rtpdump_info_t; +/****************************************************************************/ +/* init rtpstream_info_t structure */ +void rtpstream_info_init(rtpstream_info_t *info) +{ + memset(info, 0, sizeof(rtpstream_info_t)); +} + +/****************************************************************************/ +/* malloc and init rtpstream_info_t structure */ +rtpstream_info_t *rtpstream_info_malloc_and_init(void) +{ + rtpstream_info_t *dest; + + dest = (rtpstream_info_t *)g_malloc(sizeof(rtpstream_info_t)); + rtpstream_info_init(dest); + + return dest; +} + +/****************************************************************************/ +/* deep copy of rtpstream_info_t */ +void rtpstream_info_copy_deep(rtpstream_info_t *dest, const rtpstream_info_t *src) +{ + /* Deep clone of contents */ + *dest = *src; /* memberwise copy of struct */ + copy_address(&(dest->id.src_addr), &(src->id.src_addr)); + copy_address(&(dest->id.dst_addr), &(src->id.dst_addr)); + dest->payload_type_name = g_strdup(src->payload_type_name); + dest->ed137_info = g_strdup(src->ed137_info); +} + +/****************************************************************************/ +/* malloc and deep copy rtpstream_info_t structure */ +rtpstream_info_t *rtpstream_info_malloc_and_copy_deep(const rtpstream_info_t *src) +{ + rtpstream_info_t *dest; + + dest = (rtpstream_info_t *)g_malloc(sizeof(rtpstream_info_t)); + rtpstream_info_copy_deep(dest, src); + + return dest; +} + +/****************************************************************************/ +/* free rtpstream_info_t referenced values */ +void rtpstream_info_free_data(rtpstream_info_t *info) +{ + if (info->ed137_info != NULL) { + g_free(info->ed137_info); + } + if (info->payload_type_name!= NULL) { + g_free(info->payload_type_name); + } + rtpstream_id_free(&info->id); +} + +/****************************************************************************/ +/* free rtpstream_info_t referenced values and whole structure */ +void rtpstream_info_free_all(rtpstream_info_t *info) +{ + rtpstream_info_free_data(info); + g_free(info); +} + /****************************************************************************/ /* GCompareFunc style comparison function for rtpstream_info_t */ gint rtpstream_info_cmp(gconstpointer aa, gconstpointer bb) { - const rtpstream_info_t* a = (const rtpstream_info_t*)aa; - const rtpstream_info_t* b = (const rtpstream_info_t*)bb; + const rtpstream_info_t *a = (const rtpstream_info_t *)aa; + const rtpstream_info_t *b = (const rtpstream_info_t *)bb; if (a==b) return 0; @@ -82,14 +146,16 @@ gboolean rtpstream_info_is_reverse(const rtpstream_info_t *stream_a, rtpstream_i void rtpstream_reset(rtpstream_tapinfo_t *tapinfo) { GList* list; + rtpstream_info_t *stream_info; if (tapinfo->mode == TAP_ANALYSE) { /* free the data items first */ list = g_list_first(tapinfo->strinfo_list); while (list) { + stream_info = (rtpstream_info_t *)(list->data); + rtpstream_info_free_data(stream_info); g_free(list->data); - /* TODO free src_addr, dst_addr and payload_type_name? */ list = g_list_next(list); } g_list_free(tapinfo->strinfo_list); @@ -268,7 +334,7 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_, /* gather infos on the stream this packet is part of. * Addresses and strings are read-only and must be duplicated if copied. */ - memset(&new_stream_info, 0, sizeof(rtpstream_info_t)); + rtpstream_info_init(&new_stream_info); rtpstream_id_copy_pinfo(pinfo,&(new_stream_info.id),FALSE); new_stream_info.id.ssrc = rtpinfo->info_sync_src; new_stream_info.payload_type = rtpinfo->info_payload_type; @@ -279,9 +345,9 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_, list = g_list_first(tapinfo->strinfo_list); while (list) { - if (rtpstream_info_cmp(&new_stream_info, (rtpstream_info_t*)(list->data))==0) + if (rtpstream_info_cmp(&new_stream_info, (rtpstream_info_t *)(list->data))==0) { - stream_info = (rtpstream_info_t*)(list->data); /*found!*/ + stream_info = (rtpstream_info_t *)(list->data); /*found!*/ break; } list = g_list_next(list); @@ -303,12 +369,8 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_, else new_stream_info.setup_frame_number = 0xFFFFFFFF; - stream_info = g_new(rtpstream_info_t,1); - /* Deep clone of contents. */ - copy_address(&(new_stream_info.id.src_addr), &(new_stream_info.id.src_addr)); - copy_address(&(new_stream_info.id.dst_addr), &(new_stream_info.id.dst_addr)); - new_stream_info.payload_type_name = g_strdup(new_stream_info.payload_type_name); - *stream_info = new_stream_info; /* memberwise copy of struct */ + stream_info = rtpstream_info_malloc_and_init(); + rtpstream_info_copy_deep(stream_info, &new_stream_info); tapinfo->strinfo_list = g_list_prepend(tapinfo->strinfo_list, stream_info); } diff --git a/ui/tap-rtp-common.h b/ui/tap-rtp-common.h index 48186c2353..0b997b60e1 100644 --- a/ui/tap-rtp-common.h +++ b/ui/tap-rtp-common.h @@ -70,6 +70,16 @@ typedef struct _rtpstream_info_calc { guint32 last_packet_num; } rtpstream_info_calc_t; +/** + * Funcions for init and destroy of rtpstream_info_t and attached structures + */ +void rtpstream_info_init(rtpstream_info_t* info); +rtpstream_info_t *rtpstream_info_malloc_and_init(void); +void rtpstream_info_copy_deep(rtpstream_info_t *dest, const rtpstream_info_t *src); +rtpstream_info_t *rtpstream_info_malloc_and_copy_deep(const rtpstream_info_t *src); +void rtpstream_info_free_data(rtpstream_info_t* info); +void rtpstream_info_free_all(rtpstream_info_t* info); + /** * Compares two RTP stream infos (GCompareFunc style comparison function) * diff --git a/ui/voip_calls.c b/ui/voip_calls.c index 162f1af8a8..ff373506fa 100644 --- a/ui/voip_calls.c +++ b/ui/voip_calls.c @@ -50,6 +50,7 @@ #include "ui/rtp_stream.h" #include "ui/simple_dialog.h" +#include "ui/tap-rtp-common.h" #include "ui/ws_ui_util.h" #include "ui/voip_calls.h" @@ -292,8 +293,7 @@ voip_calls_reset_all_taps(voip_calls_tapinfo_t *tapinfo) while(list) { strinfo = (rtpstream_info_t *)list->data; - wmem_free(NULL, strinfo->payload_type_name); - wmem_free(NULL, strinfo->ed137_info); + rtpstream_info_free_data(strinfo); list = g_list_next(list); } g_list_free(tapinfo->rtpstream_list); @@ -555,11 +555,14 @@ rtp_reset(void *tap_offset_ptr) { voip_calls_tapinfo_t *tapinfo = tap_id_to_base(tap_offset_ptr, tap_id_offset_rtp_); GList *list; + rtpstream_info_t *stream_info; /* free the data items first */ list = g_list_first(tapinfo->rtpstream_list); while (list) { + stream_info = (rtpstream_info_t*)(list->data); + rtpstream_info_free_data(stream_info); g_free(list->data); list = g_list_next(list); } @@ -630,7 +633,7 @@ rtp_packet(void *tap_offset_ptr, packet_info *pinfo, epan_dissect_t *edt, void c /* not in the list? then create a new entry */ if (strinfo==NULL) { - strinfo = (rtpstream_info_t *)g_malloc0(sizeof(rtpstream_info_t)); + strinfo = rtpstream_info_malloc_and_init(); rtpstream_id_copy_pinfo(pinfo,&(strinfo->id),FALSE); strinfo->id.ssrc = rtp_info->info_sync_src; strinfo->payload_type = rtp_info->info_payload_type; @@ -685,7 +688,7 @@ rtp_draw(void *tap_offset_ptr) { voip_calls_tapinfo_t *tapinfo = tap_id_to_base(tap_offset_ptr, tap_id_offset_rtp_); GList *rtpstreams_list; - rtpstream_info_t *rtp_listinfo; + rtpstream_info_t *rtp_listinfo; /* GList *voip_calls_graph_list; */ seq_analysis_item_t *gai = NULL; seq_analysis_item_t *new_gai; @@ -758,7 +761,7 @@ rtp_packet_draw(void *tap_offset_ptr) { voip_calls_tapinfo_t *tapinfo = tap_id_to_base(tap_offset_ptr, tap_id_offset_rtp_); GList *rtpstreams_list; - rtpstream_info_t *rtp_listinfo; + rtpstream_info_t *rtp_listinfo; GList *voip_calls_graph_list; guint item; seq_analysis_item_t *gai;