[This response might have been invalidated by v4]
W dniu 01.08.2016 o 14:00, Lars Schneider pisze:
>> On 30 Jul 2016, at 12:49, Jakub Narębski <jnareb@xxxxxxxxx> wrote:
>> W dniu 30.07.2016 o 01:37, larsxschneider@xxxxxxxxx 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?
Yes, I think it is more clear.
The only thing that could be improved is to perhaps instead of using
"then a proper pkt-line buffer with header and data section can be created"
it might be more clear to write
"then a proper pkt-line buffer with data section and a place for pkt-line header"
>>> +{
>>> + 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 ?
Did you mean
+ char hdr[PKTLINE_HEADER_LEN];
+ set_packet_header(hdr, sizeof(hdr) + size);
>>> + !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?!
This is true both for || and for &&, as in C logical boolean operators
short-circuit.
> Should I make this more explicit with a if clause?
No need.
--
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