From: Jakub Narębski Date: Wed, 03 Aug 2016 20:15:12 GMT Subject: Re: [PATCH v3 04/10] pkt-line: call packet_trace() only if a packet is actually send Message-ID: In-Reply-To: <52A5703D-AF9E-405D-A4F8-E30A7B923400@gmail.com> [This response might have been invalidated by v4] W dniu 01.08.2016 o 14:18, Lars Schneider pisze: >> On 30 Jul 2016, at 14:29, Jakub Narębski wrote: >> W dniu 30.07.2016 o 01:37, larsxschneider@gmail.com pisze: >> I don't buy this explanation. If you want to trace packets, you might >> do it on input (when formatting packet), or on output (when writing >> packet). It's when there are more than one formatting function, but >> one writing function, then placing trace call in write function means >> less code duplication; and of course the reverse. >> >> Another issue is that something may happen between formatting packet >> and sending it, and we probably want to packet_trace() when packet >> is actually send. >> >> Neither of those is visible in commit message. > > The packet_trace() call in format_packet() is not ideal, as we would print > a trace when a packet is formatted and (potentially) when the same packet is > actually written. This was no problem up until now because packet_write(), > the function that uses format_packet() and writes the formatted packet, > did not trace the packet. > > This developer believes that trace calls should only happen when a packet > is actually written as the packet could be modified between formatting > and writing. Therefore the trace call was moved from format_packet() to > packet_write(). > > -- > > Better? Yes, that's much better. P.S. Yes, this is one of those changes where commit message is much longer than the change itself... -- Jakub Narębski