> On 29 Jul 2016, at 13:24, Jakub Narębski <jnareb@xxxxxxxxx> wrote: > > W dniu 2016-07-29 o 12:38, Lars Schneider pisze: >> On 27 Jul 2016, at 11:41, Eric Wong <e@xxxxxxxxx> wrote: >>> larsxschneider@xxxxxxxxx wrote: > >>>> +static off_t multi_packet_read(struct strbuf *sb, const int fd, const size_t size) >>> >>> I'm no expert in C, but this might be const-correctness taken >>> too far. I think basing this on the read(2) prototype is less >>> surprising: >>> >>> static ssize_t multi_packet_read(int fd, struct strbuf *sb, size_t size) >> >> Hm... ok. I like `const` because I think it is usually easier to read/understand >> functions that do not change their input variables. This way I can communicate >> my intention to future people modifying this function! > > Well, scalar types like `size_t` are always passed by value, so here `const` > doesn't matter, and it makes line longer. I think library functions do not > use `const` for `size_t` parameters. > > You are reading from the file descriptor `fd`, so it state would change. > Using `const` feels a bit like lying. Also, it is scalar type. OK, since you are the second reviewer arguing against `const` I will remove it. > > [...] >> I agree with your reordering of the parameters, though! >> >> Speaking of coding style... convert.c is already big and gets only bigger >> with this patch (1720 lines). Would it make sense to add a new file >> "convert-pipe-protocol.c" >> or something for my additions? > > I wonder if it would be possible to enhance existing functions, instead > of redoing them (at least in part) for per-command filter driver protocol. I think I reused as much as possible. 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