Re: [Patch size_t V3 17/19] Convert ref-filter to size_t

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2017, 18:03 UTC
Message-ID: <xmqqmv6ygjha.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1502914591-26215-18-git-send-email-martin@mail.zuhause>

Martin Koegler <martin.koegler@chello.at> writes:

Show 5 quoted lines

> -static char *copy_subject(const char *buf, unsigned long len)
> +static char *copy_subject(const char *buf, size_t len)
>  {
>  	char *r = xmemdupz(buf, len);
>  	int i;

This has the same "we are still iterating with 'int i' over an array that can be 'size_t len' long" issue as I pointed out in 19/19. The remedy is similar, i.e.

	static char *copy_subject(const char *buf, size_t len)
	{
		char *ret = xmemdupz(buf, len);
		char *cp;

		for (cp = ret; cp < ret + len; cp++)
			if (*cp == '\n')
				*cp = ' ';
		return ret;
	}

which should come either before the whole series or after the series settles, not inside this series.

I do also wonder if we want to do more for other control characters (e.g. CR, HT, etc.) in this function, though. That is totally outside the scope of this series and should be done independently.

Show 9 quoted lines

> @@ -898,7 +898,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
>  }
>  
>  /* See grab_values */
> -static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
> +static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, size_t sz)
>  {
>  	int i;
>  	int wholen = strlen(who);

Shouldn't this also become size_t, I wonder?

I did not see anything fishy in the remaining parts I did not comment on.

Thanks.