> On 03 Aug 2016, at 22:12, Jakub Narębski <jnareb@xxxxxxxxx> wrote: > > [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" OK. I changed it to "If the caller has control over the buffer creation then a proper pkt-line buffer with header and data section can be allocated. 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)..." However, I am not yet sure if I can/will keep this patch: http://public-inbox.org/git/xmqqeg645x6b.fsf%40gitster.mtv.corp.google.com/ > >>>> +{ >>>> + 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); yes! >>>> + !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. True. That's why I did not get your "de morganize" it comment... what would de morgan change? > >> Should I make this more explicit with a if clause? > > No need. OK Thanks, Lars -- 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