Re: [PATCH v3 02/10] pkt-line: add direct_packet_write() and direct_packet_write_data()

>> On 30 Jul 2016, at 12:49, Jakub Narębski <jnareb@gmail.com> wrote:
>> W dniu 30.07.2016 o 01:37, larsxschneider@gmail.com pisze:
>>>
>>> Sometimes pkt-line data is already available in a buffer and it would
>>> be a waste of resources to write the packet using packet_write() which
>>> would copy the existing buffer into a strbuf before writing it.
>>>
>>> If the caller has control over the buffer creation then the
>>> PKTLINE_DATA_START macro can be used to skip the header and write
>>> directly into the data section of a pkt-line (PKTLINE_DATA_LEN bytes
>>> would be the maximum). direct_packet_write() would take this buffer,
>>> adjust the pkt-line header and write it.
>>>
>>> If the caller has no control over the buffer creation then
>>> direct_packet_write_data() can be used. This function creates a pkt-line
>>> header. Afterwards the header and the data buffer are written using two
>>> consecutive write calls.
>>
>> I don't quite understand what do you mean by "caller has control
>> over the buffer creation".  Do you mean that caller either can write
>> over the buffer, or cannot overwrite the buffer?  Or do you mean that
>> caller either can allocate buffer to hold header, or is getting
>> only the data?
> 
> How about this:
> 
> [...]
> 
> If the caller creates the buffer then a proper pkt-line buffer with header
> and data section can be created. The PKTLINE_DATA_START macro can be used 
> to skip the header section and write directly to the data section (PKTLINE_DATA_LEN 
> bytes would be the maximum). direct_packet_write() would take this buffer, 
> fill the pkt-line header section with the appropriate data length value and 
> write the entire buffer.
> 
> If the caller does not create the buffer, and consequently cannot leave room
> for the pkt-line header, then direct_packet_write_data() can be used. This 
> function creates an extra buffer for the pkt-line header and afterwards writes
> the header buffer and the data buffer with two consecutive write calls.
> 
> ---
> Is that more clear?

>>> +{
>>> +	int ret = 0;
>>> +	char hdr[4];
>>> +	set_packet_header(hdr, sizeof(hdr) + size);
>>> +	packet_trace(buf, size, 1);
>>> +	if (gentle) {
>>> +		ret = (
>>> +			!write_or_whine_pipe(fd, hdr, sizeof(hdr), "pkt-line header") ||
>>
>> You can write '4' here, no need for sizeof(hdr)... though compiler would
>> optimize it away.
> 
> Right, it would be optimized. However, I don't like the 4 there either. OK to use a macro
> instead? PKTLINE_HEADER_LEN ?

>>> +			!write_or_whine_pipe(fd, buf, size, "pkt-line data")
>>> +		);
>>
>> Do we want to try to write "pkt-line data" if "pkt-line header" failed?
>> If not, perhaps De Morgan-ize it
>>
>>  +		ret = !(
>>  +			write_or_whine_pipe(fd, hdr, sizeof(hdr), "pkt-line header") &&
>>  +			write_or_whine_pipe(fd, buf, size, "pkt-line data")
>>  +		);
> 
> 
> Original:
> 		ret = (
> 			!write_or_whine_pipe(fd, hdr, sizeof(hdr), "pkt-line header") ||
> 			!write_or_whine_pipe(fd, data, size, "pkt-line data")
> 		);
> 
> Well, if the first write call fails (return == 0), then it is negated and evaluates to true.
> I would think the second call is not evaluated, then?!