Web lists-archives.com

Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long




Am 19.11.2018 um 06:33 schrieb Torsten Bögershausen:
> The archive-tar.c is actually a good example, why a step-by-step update
> is not ideal (the code would not work any more on Win64).
> 
> If we look here:
> 
> static int stream_blocked(const struct object_id *oid)
> {
> 	struct git_istream *st;
> 	enum object_type type;
> 	size_t sz;
> 	char buf[BLOCKSIZE];
> 	ssize_t readlen;
> 
> 	st = open_istream(oid, &type, &sz, NULL);
>                                       ^^^^^
> 	if (!st)
> 		return error(_("cannot stream blob %s"), oid_to_hex(oid));
> 	for (;;) {
> 
> The sz variable must follow whatever open_istream() uses, so if we start
> with archive-tar.c, we must use either size_t or ulong, whatever
> open_istream() needs. Otherwise things will break:
> archive-tar.c uses ulong, open_istream() size_t, but we are passing pointers
> around, and here &ulong != &size_t
> 
> If we only update open_istream(), but not archive-tar.c, then
> things are not better:
> &size_t != &ulong.
> 
> I don't have a good idea how to split the patch.

sz is not actually used later in that function; this change can
be done independently of any other ulong/size_t conversion in that
file.

Hmm, looking at that call I wonder why open_istream() doesn't return
type and size in the struct git_istream.  To make it match
read_object_file(), I suppose.  Perhaps it's an opportunity to
improve its interface?

René