Re: [RFC] Don't expect verify_pack() callers to set pack_size

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

 



Mike Hommey <mh@xxxxxxxxxxxx> wrote:
> Since use_pack() will end up populating pack_size if it is not already set,
> we can just adapt the code in verify_packfile() such that it doesn't require
> pack_size to be set beforehand.
> 
> This allows callers not to have to set pack_size themselves, and we can thus
> revert changes from 1c23d794.

That's a good improvement in code clarity.  When I implemented
1c23 I wasn't sure what was initialized in the struct packed_git
that the HTTP code was slugging around in request->userData, and
it was hard to track down.  1c23 was a quick fix to get HTTP fetch
working again when sp/mmap merged into next.

I'm guessing you have figured out it is not actually open at this
point, which makes sense as we did fclose() calls to close out
the writer.  So sha1_file.c needs to open a new file descriptor.
use_pack() will force it to be opened and the open packfile function
does update the pack_size if we don't have it yet.

Acked-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>

> diff --git a/pack-check.c b/pack-check.c
> index d7dd62b..e7f0126 100644
> --- a/pack-check.c
> +++ b/pack-check.c
> @@ -37,14 +37,16 @@ static int verify_packfile(struct packed_git *p,
>  	 */
>  
>  	SHA1_Init(&ctx);
> -	while (offset < pack_sig) {
> +	do {
>  		unsigned int remaining;
>  		unsigned char *in = use_pack(p, w_curs, offset, &remaining);
>  		offset += remaining;
> +		if (pack_sig == 0)
> +			pack_sig = p->pack_size - 20;

fyi, we prefer "if (!pack_sig)" in Git for equality with 0 tests.

-- 
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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