Re: [PATCH v3 2/2] name-rev.c: use strbuf_getline instead of limited size buffer

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

 



"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

> From: John Cai <johncai86@xxxxxxxxx>
>
> Using a buffer limited to 2048 is unnecessarily limiting. Switch to
> using a string buffer to read in stdin for annotation.
>
> Signed-off-by: "John Cai" <johncai86@xxxxxxxxx>
> ---
>  builtin/name-rev.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 21370afdaf9..85993bc2b38 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -625,12 +625,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
>  	if (annotate_stdin) {
>  		struct strbuf sb = STRBUF_INIT;
>  
> -		while (!feof(stdin)) {
> -			char *p = fgets(buffer, sizeof(buffer), stdin);
> -			if (!p)
> -				break;
> -			name_rev_line(p, &data);
> +		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {

As we are reading text, I wonder if it makes more sense to just use
strbuf_getline() that is meant to use the definition of "line" that
honors the platform convention (i.e. on Windows, '\r\n' is taken as
the EOL marker).

> +			name_rev_line(sb.buf, &data);
>  		}
> +		strbuf_release(&sb);
>  	} else if (all) {
>  		int i, max;

Other than that, looking good.

Thanks.



[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]

  Powered by Linux