diff --git a/editcap.c b/editcap.c index 495ab6d1d9..c1616db61e 100644 --- a/editcap.c +++ b/editcap.c @@ -1945,7 +1945,8 @@ main(int argc, char *argv[]) if (comment != NULL) { /* Copy and change rather than modify returned rec */ temp_rec = *rec; - temp_rec.opt_comment = g_strdup(comment); + /* The comment is not modified by dumper, cast away. */ + temp_rec.opt_comment = (char *)comment; temp_rec.has_comment_changed = TRUE; rec = &temp_rec; } else { diff --git a/epan/wslua/wslua_dumper.c b/epan/wslua/wslua_dumper.c index 64e4a6affd..1d9fa36ba5 100644 --- a/epan/wslua/wslua_dumper.c +++ b/epan/wslua/wslua_dumper.c @@ -440,6 +440,11 @@ WSLUA_METHOD Dumper_dump_current(lua_State* L) { rec.rec_header.packet_header.pkt_encap = lua_pinfo->rec->rec_header.packet_header.pkt_encap; rec.rec_header.packet_header.pseudo_header = *lua_pinfo->pseudo_header; + /* + * wtap_dump does not modify rec.opt_comment, so it should be possible to + * pass epan_get_user_comment() or lua_pinfo->rec->opt_comment directly. + * Temporarily duplicating the memory should not hurt though. + */ if (lua_pinfo->fd->has_user_comment) { rec.opt_comment = wmem_strdup(wmem_packet_scope(), epan_get_user_comment(lua_pinfo->epan, lua_pinfo->fd)); rec.has_comment_changed = TRUE; diff --git a/epan/wslua/wslua_frame_info.c b/epan/wslua/wslua_frame_info.c index e283bd4459..fa6359c6cf 100644 --- a/epan/wslua/wslua_frame_info.c +++ b/epan/wslua/wslua_frame_info.c @@ -207,6 +207,7 @@ WSLUA_ATTRIBUTE_NAMED_NUMBER_SETTER(FrameInfo,original_length,rec->rec_header.pa WSLUA_ATTRIBUTE_NAMED_NUMBER_GETTER(FrameInfo,encap,rec->rec_header.packet_header.pkt_encap); WSLUA_ATTRIBUTE_NAMED_NUMBER_SETTER(FrameInfo,encap,rec->rec_header.packet_header.pkt_encap,int); +// XXX rec->opt_comment is glib-allocated memory, rec is wtap_rec. What frees it? /* WSLUA_ATTRIBUTE FrameInfo_comment RW A string comment for the packet, if the `wtap_presence_flags.COMMENTS` was set in the presence flags; nil if there is no comment. */ WSLUA_ATTRIBUTE_NAMED_STRING_GETTER(FrameInfo,comment,rec->opt_comment); diff --git a/ui/tap_export_pdu.c b/ui/tap_export_pdu.c index 5d5a11986f..b2a299b74a 100644 --- a/ui/tap_export_pdu.c +++ b/ui/tap_export_pdu.c @@ -54,6 +54,9 @@ export_pdu_packet(void *tapdata, packet_info *pinfo, epan_dissect_t *edt, const rec.rec_header.packet_header.pkt_encap = exp_pdu_tap_data->pkt_encap; + /* rec.opt_comment is not modified by wtap_dump, but if for some reason the + * epan_get_user_comment() or pinfo->rec->opt_comment are invalidated, + * copying it here does not hurt. (Can invalidation really happen?) */ if (pinfo->fd->has_user_comment) { rec.opt_comment = g_strdup(epan_get_user_comment(edt->session, pinfo->fd)); rec.has_comment_changed = TRUE; diff --git a/wiretap/erf.c b/wiretap/erf.c index 45884da253..03a0419a6f 100644 --- a/wiretap/erf.c +++ b/wiretap/erf.c @@ -2257,6 +2257,9 @@ static int erf_update_anchors_from_header(erf_t *erf_priv, wtap_rec *rec, union } if (comment) { + /* Will be freed by either wtap_sequential_close (for rec = &wth->rec) or by + * the caller of wtap_seek_read. See wtap_rec_cleanup. */ + g_free(rec->opt_comment); rec->opt_comment = g_strdup(comment); rec->presence_flags |= WTAP_HAS_COMMENTS; } else { @@ -2264,8 +2267,7 @@ static int erf_update_anchors_from_header(erf_t *erf_priv, wtap_rec *rec, union * Need to set opt_comment to NULL to prevent other packets * from displaying the same comment */ - /* XXX: We cannot free the old comment because it can be for a different - * frame and still in use, wiretap should be handling this better! */ + g_free(rec->opt_comment); rec->opt_comment = NULL; } diff --git a/wiretap/nettrace_3gpp_32_423.c b/wiretap/nettrace_3gpp_32_423.c index eb41e2ba8b..1bc2c15490 100644 --- a/wiretap/nettrace_3gpp_32_423.c +++ b/wiretap/nettrace_3gpp_32_423.c @@ -171,7 +171,9 @@ nettrace_read(wtap *wth, int *err, gchar **err_info, gint64 *data_offset) wth->rec.rec_header.packet_header.pkt_encap = file_info->wth_tmp_file->rec.rec_header.packet_header.pkt_encap; wth->rec.tsprec = file_info->wth_tmp_file->rec.tsprec; wth->rec.rec_header.packet_header.interface_id = file_info->wth_tmp_file->rec.rec_header.packet_header.interface_id; + /* Steal memory from the pcapng wth. */ wth->rec.opt_comment = file_info->wth_tmp_file->rec.opt_comment; + file_info->wth_tmp_file->rec.opt_comment = NULL; wth->rec.rec_header.packet_header.drop_count = file_info->wth_tmp_file->rec.rec_header.packet_header.drop_count; wth->rec.rec_header.packet_header.pack_flags = file_info->wth_tmp_file->rec.rec_header.packet_header.pack_flags; diff --git a/wiretap/pcapng.c b/wiretap/pcapng.c index 23e554830f..bd101b4722 100644 --- a/wiretap/pcapng.c +++ b/wiretap/pcapng.c @@ -1323,6 +1323,7 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, pcapng_t *pn, wta } /* Option defaults */ + g_free(wblock->rec->opt_comment); /* Free memory from an earlier read. */ wblock->rec->opt_comment = NULL; wblock->rec->rec_header.packet_header.drop_count = -1; wblock->rec->rec_header.packet_header.pack_flags = 0; @@ -1374,6 +1375,7 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, pcapng_t *pn, wta case(OPT_COMMENT): if (oh->option_length > 0 && oh->option_length < opt_cont_buf_len) { wblock->rec->presence_flags |= WTAP_HAS_COMMENTS; + g_free(wblock->rec->opt_comment); wblock->rec->opt_comment = g_strndup((char *)option_content, oh->option_length); pcapng_debug("pcapng_read_packet_block: length %u opt_comment '%s'", oh->option_length, wblock->rec->opt_comment); } else { @@ -1571,6 +1573,7 @@ pcapng_read_simple_packet_block(FILE_T fh, pcapng_block_header_t *bh, pcapng_t * wblock->rec->ts.secs = 0; wblock->rec->ts.nsecs = 0; wblock->rec->rec_header.packet_header.interface_id = 0; + g_free(wblock->rec->opt_comment); /* Free memory from an earlier read. */ wblock->rec->opt_comment = NULL; wblock->rec->rec_header.packet_header.drop_count = 0; wblock->rec->rec_header.packet_header.pack_flags = 0; diff --git a/wiretap/wtap.c b/wiretap/wtap.c index 693999bbce..fb77a4b23e 100644 --- a/wiretap/wtap.c +++ b/wiretap/wtap.c @@ -1460,6 +1460,8 @@ wtap_rec_init(wtap_rec *rec) void wtap_rec_cleanup(wtap_rec *rec) { + g_free(rec->opt_comment); + rec->opt_comment = NULL; ws_buffer_free(&rec->options_buf); }