Re: [PATCH v5 02/15] pkt-line: call packet_trace() only if a packet is actually send

On Wed, Aug 10, 2016 at 03:24:38PM +0200, Lars Schneider wrote:

quoted

On Wed, Aug 10, 2016 at 03:03:58PM +0200, larsxschneider@gmail.com wrote:

quoted

From: Lars Schneider <redacted>

The packet_trace() call is not ideal in format_packet() as we would print
a trace when a packet is formatted and (potentially) when the packet is
actually send. This was no problem up until now because format_packet()
was only used by one function. Fix it by moving the trace call into the
function that actually sends the packet.

It looks like there are two functions: packet_write() and
packet_buf_write().

I did not call trace in packet_buf_write() because this function does not
perform any writes.

Yes, but then who is responsible for the trace? The caller?

And why is it a bad thing to do it some time other than writing? It is
if you format and then _don't_ write the packet, but the current callers
are not doing that.

quoted

Your patch only touches one of them, and it looks like we would fail to
trace many packets (e.g., see receive-pack.c:report(), which uses
packet_buf_write() and then write()s out the result).

I see. But isn't it confusing if packet_buf_write() issues a trace call?
If I just call this function then nothing happens at all. Shouldn't the
trace call be made in receive-pack.c:report() ? Or shouldn't receive-pack
let pkt-line.c perform the write calls?

How would report() do that without re-parsing each of the packets?

-Peff