Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Lars,

W dniu 20.09.2016 o 21:02, larsxschneider@xxxxxxxxx pisze:

> From: Lars Schneider <larsxschneider@xxxxxxxxx>
> 
> 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 <gitster@xxxxxxxxx>

Just so nobody wonders later why this patch was needed/suggested.

> Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx>
> ---
>  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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]