Re: [PATCH v3 04/10] pkt-line: call packet_trace() only if a packet is actually send

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[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 <jnareb@xxxxxxxxx> wrote:
>> W dniu 30.07.2016 o 01:37, larsxschneider@xxxxxxxxx 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]