Changeset 1935565

Timestamp:

11/29/12 10:26:27 (9 years ago)

Author:

Shane Alcock <salcock@…>

Branches:

4.0.1-hotfixes, cachetimestamps, develop, dpdk-ndag, etsilive, getfragoff, help, libtrace4, master, ndag_format, pfring, rc-4.0.1, rc-4.0.2, rc-4.0.3, rc-4.0.4, ringdecrementfix, ringperformance, ringtimestampfixes

Children:

f240823

Parents:

17dc71c

Message:

• Fixed double free bug when using trace_event to read from a trace file. Previously, if you tried to clean up properly after finishing with trace_event, you would get a double free bug because the packet buffer was being freed twice: once during trace_destroy() (because of the internal packet stored by trace_event) and again during trace_destroy_packet().
• Removed some of the unused code from trace_event_trace in format_helper.c
• Make sure we only free packet->buffer during trace_destroy_packet if it is not NULL

Location:

lib

Files:

• format_helper.c (modified) (2 diffs)
• trace.c (modified) (2 diffs)

lib/format_helper.c

@@ -216 +216 @@
         }
 
-        /* This is the first packet, so just fire away. */
-        /* TODO: finalise packet */
-        
-        /* XXX: Could we do this more efficiently? */
-
-        /* We do a lot of freeing and creating of packet buffers with this
-         * method, but at least it works unlike what was here previously */
-        //if (packet->buf_control == TRACE_CTRL_PACKET) {
-        //      free(packet->buffer);
-        //}
-        
         /* The packet that we had read earlier is now ready to be returned
          * to the user - switch all the pointers etc. over */   
@@ -236 +225 @@
         packet->buffer = trace->event.packet->buffer;
         packet->buf_control = trace->event.packet->buf_control;
-
-        //trace->event.packet->buffer = NULL;
-        //trace->event.packet->buf_control = TRACE_CTRL_EXTERNAL;
-        
-        //trace_destroy_packet(trace->event.packet);
-        //trace->event.packet = NULL;
 
         event.type = TRACE_EVENT_PACKET;

lib/trace.c

@@ -592 +592 @@
         if (libtrace->uridata)
                 free(libtrace->uridata);
-        if (libtrace->event.packet)
-                trace_destroy_packet(libtrace->event.packet);
+        if (libtrace->event.packet) {
+                /* Don't use trace_destroy_packet here - there is almost
+                 * certainly going to be another libtrace_packet_t that is
+                 * pointing to the buffer for this packet, so we don't want
+                 * to free it. Rather, it will get freed when the user calls
+                 * trace_destroy_packet on the libtrace_packet_t that they
+                 * own.
+                 *
+                 * All we need to do then is free our packet structure itself.
+                 */
+                 free(libtrace->event.packet);
+        }
         free(libtrace);
 }
@@ -664 +674 @@
  */
 DLLEXPORT void trace_destroy_packet(libtrace_packet_t *packet) {
-        if (packet->buf_control == TRACE_CTRL_PACKET) {
+        if (packet->buf_control == TRACE_CTRL_PACKET && packet->buffer) {
                 free(packet->buffer);
         }