Omar Polo <op@omarpolo.com> wrote:
> Finally managed to understand why the gotd regress was was consistently
> failing on my buildbot.  The issue is here:
> 
> lib/pack_index.c:
> >806                 if (lseek(pack->fd, -digest_len, SEEK_END) == -1) {
>  807                         err = got_error_from_errno("lseek");
>  808                         goto done;
>  809                 }
>  810                 n = read(pack->fd, pack_hash_expected, digest_len);
> 
> My interpretation is that `-digest_len' (which is a size_t so 32bit on
> x86) wraps and then gets extended to a off_t (64 bit i believe), so we
> actually seek very, very far past the end of the file and then read(2)
> returns zero.
> 
> Originally we did -SHA1_DIGEST_LENGTH, which is fine, but a careless
> substitution with `digest_len' plus testing only on 64 bit arches has
> hidden this problem.
> 
> With this my buildbot seems happy, gotd regress passes fully, cmdline is
> still running but seems fine (takes a looong time :-) and tog mostly
> passes (there's a failure in one test but seems independent from this,
> i'll look into it and post another thread.)
> 
> ok?

Your diagnosis sounds right to me.

A tiny style nit (annotated inline) that was already there which you
might want to touch up while here but ok either way.

> diff /home/op/w/got
> commit - faf51db5e8152629d9c4aa4672b3f26e6acecf10
> path + /home/op/w/got
> blob - f5c3ffa28603a655efaca764f0ff6031de2ef9f5
> file + lib/pack_index.c
> --- lib/pack_index.c
> +++ lib/pack_index.c
> @@ -627,7 +627,7 @@ got_pack_index(struct got_pack *pack, int idxfd, FILE 
>  	size_t mapoff = 0;
>  	int p_indexed = 0, last_p_indexed = -1;
>  	int p_resolved = 0, last_p_resolved = -1;
> -	size_t digest_len = got_hash_digest_length(pack->algo);
> +	ssize_t digest_len = got_hash_digest_length(pack->algo);

changed line could remove function call in initialiser:

	ssize_t digest_len;

	digest_len = got_hash_digest_length(pack->algo);

>  
>  	/* Require that pack file header and hash trailer are present. */
>  	if (pack->filesize < sizeof(hdr) + digest_len)


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68