Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()
W dniu 26.09.2016 o 20:49, Lars Schneider pisze: > On 24 Sep 2016, at 23:14, Jakub Narębski wrote: >> W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze: >> >>> From: Lars Schneider >>> >>> packet_write() should be called packet_write_fmt() as the string >>> parameter can be formatted. >> >> I would say: >> >> packet_write() should be called packet_write_fmt() because it >> is printf-like function where first parameter is format string. >> >> Or something like that. But such minor change might be not worth >> yet another reroll of this patch series. >> >> Perhaps it would be a good idea to explain the reasoning behind >> this change: >> >> This is important distinction to know from the name if the >> function accepts arbitrary binary data and/or arbitrary >> strings to be written - packet_write[_fmt()] do not. > > packet_write() should be called packet_write_fmt() because it is a > printf-like function that takes a format string as first parameter. > > packet_write_fmt() should be used for text strings only. Arbitrary > binary data should use a new packet_write() function that is introduced > in a subsequent patch. > > Better? Better. > >>> pkt-line.h | 2 +- >>> shallow.c| 2 +- >>> upload-pack.c| 30 +++--- >>> 11 files changed, 29 insertions(+), 29 deletions(-) >> >> Diffstat looks correct. Was the patch generated by doing search >> and replace? > > Yes. Good. -- Jakub Narębski
Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()
On 24 Sep 2016, at 23:14, Jakub Narębski wrote: > Hello Lars, > > W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze: > >> From: Lars Schneider >> >> packet_write() should be called packet_write_fmt() as the string >> parameter can be formatted. > > I would say: > > packet_write() should be called packet_write_fmt() because it > is printf-like function where first parameter is format string. > > Or something like that. But such minor change might be not worth > yet another reroll of this patch series. > > Perhaps it would be a good idea to explain the reasoning behind > this change: > > This is important distinction to know from the name if the > function accepts arbitrary binary data and/or arbitrary > strings to be written - packet_write[_fmt()] do not. packet_write() should be called packet_write_fmt() because it is a printf-like function that takes a format string as first parameter. packet_write_fmt() should be used for text strings only. Arbitrary binary data should use a new packet_write() function that is introduced in a subsequent patch. Better? >> pkt-line.h | 2 +- >> shallow.c| 2 +- >> upload-pack.c| 30 +++--- >> 11 files changed, 29 insertions(+), 29 deletions(-) > > Diffstat looks correct. Was the patch generated by doing search > and replace? Yes. - Lars
Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()
Hello Lars, W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze: > From: Lars Schneider > > packet_write() should be called packet_write_fmt() as the string > parameter can be formatted. I would say: packet_write() should be called packet_write_fmt() because it is printf-like function where first parameter is format string. Or something like that. But such minor change might be not worth yet another reroll of this patch series. Perhaps it would be a good idea to explain the reasoning behind this change: This is important distinction to know from the name if the function accepts arbitrary binary data and/or arbitrary strings to be written - packet_write[_fmt()] do not. > > Suggested-by: Junio C Hamano Just so nobody wonders later why this patch was needed/suggested. > Signed-off-by: Lars Schneider > --- > builtin/archive.c| 4 ++-- > builtin/receive-pack.c | 4 ++-- > builtin/remote-ext.c | 4 ++-- > builtin/upload-archive.c | 4 ++-- > connect.c| 2 +- > daemon.c | 2 +- > http-backend.c | 2 +- > pkt-line.c | 2 +- The header of the renamed function looks now very nice: void packet_write_fmt(int fd, const char *fmt, ...) ^^^ ^^^ > pkt-line.h | 2 +- > shallow.c| 2 +- > upload-pack.c| 30 +++--- > 11 files changed, 29 insertions(+), 29 deletions(-) Diffstat looks correct. Was the patch generated by doing search and replace? Best, -- Jakub Narębski
