Re: [PATCH v2 1/1] Upcast size_t variables to uintmax_t when printing

2018-11-12 Thread Jeff King
On Sun, Nov 11, 2018 at 11:16:50AM +0100, Torsten Bögershausen wrote:

> > I like the overall direction. I feel a little funny doing this step now,
> > and not as part of a series to convert individual variables. But I
> > cannot offhand think of any reason that it would behave badly even if
> > the other part does not materialize
> > 
> 
> Hej all,
> There may be some background information missing:
> - I did a 2-patch series based on this commit in pu:
> commit 37c59c3e8fac8bae7ccc5baa148b0e9bae0c8d65
> Author: Junio C Hamano 
> Date:   Sat Oct 27 16:42:25 2018 +0900
> 
> treewide: apply cocci patch
> 
> (that patch was never send out, see below)
> 
> The week later, I tried to apply it on pu, but that was nearly hopeless,
> as too much things had changed on pu.
> I had the chance to compile & test it, but decided to take "part2" before
> "part1", so to say:
> Fix all the printing, and wait for the master branch to settle,
> and then do the "unsigned long" -> size_t conversion.
> That will probably happen after 2.20.

Ah, OK. I am fine with that approach. My thinking was that we'd see
individual functions and their callers converted, which is another way
to do it incrementally. But sometimes that ends up cascading and you end
up having to change quite a bit of the callstack anyway.

-Peff


Re: [PATCH v2 1/1] Upcast size_t variables to uintmax_t when printing

2018-11-11 Thread Torsten Bögershausen
On Sun, Nov 11, 2018 at 02:28:35AM -0500, Jeff King wrote:
> On Sun, Nov 11, 2018 at 08:05:04AM +0100, tbo...@web.de wrote:
> 
> > From: Torsten Bögershausen 
> > 
> > When printing variables which contain a size, today "unsigned long"
> > is used at many places.
> > In order to be able to change the type from "unsigned long" into size_t
> > some day in the future, we need to have a way to print 64 bit variables
> > on a system that has "unsigned long" defined to be 32 bit, like Win64.
> > 
> > Upcast all those variables into uintmax_t before they are printed.
> > This is to prepare for a bigger change, when "unsigned long"
> > will be converted into size_t for variables which may be > 4Gib.
> 
> I like the overall direction. I feel a little funny doing this step now,
> and not as part of a series to convert individual variables. But I
> cannot offhand think of any reason that it would behave badly even if
> the other part does not materialize
> 

Hej all,
There may be some background information missing:
- I did a 2-patch series based on this commit in pu:
commit 37c59c3e8fac8bae7ccc5baa148b0e9bae0c8d65
Author: Junio C Hamano 
Date:   Sat Oct 27 16:42:25 2018 +0900

treewide: apply cocci patch

(that patch was never send out, see below)

The week later, I tried to apply it on pu, but that was nearly hopeless,
as too much things had changed on pu.
I had the chance to compile & test it, but decided to take "part2" before
"part1", so to say:
Fix all the printing, and wait for the master branch to settle,
and then do the "unsigned long" -> size_t conversion.
That will probably happen after 2.20.

At the moment, the big "unsigned long" -> size_t conversion is dependend on
 - mk/use-size-t-in-zlib
 - sb/more-repo-in-api,
 - jk/xdiff-interface'
 (and probably more stuff from Duy and others)

How should we handle the big makeover ?

> -Peff

And thanks for reading my stuff


Re: [PATCH v2 1/1] Upcast size_t variables to uintmax_t when printing

2018-11-10 Thread Jeff King
On Sun, Nov 11, 2018 at 08:05:04AM +0100, tbo...@web.de wrote:

> From: Torsten Bögershausen 
> 
> When printing variables which contain a size, today "unsigned long"
> is used at many places.
> In order to be able to change the type from "unsigned long" into size_t
> some day in the future, we need to have a way to print 64 bit variables
> on a system that has "unsigned long" defined to be 32 bit, like Win64.
> 
> Upcast all those variables into uintmax_t before they are printed.
> This is to prepare for a bigger change, when "unsigned long"
> will be converted into size_t for variables which may be > 4Gib.

I like the overall direction. I feel a little funny doing this step now,
and not as part of a series to convert individual variables. But I
cannot offhand think of any reason that it would behave badly even if
the other part does not materialize

-Peff


[PATCH v2 1/1] Upcast size_t variables to uintmax_t when printing

2018-11-10 Thread tboegi
From: Torsten Bögershausen 

When printing variables which contain a size, today "unsigned long"
is used at many places.
In order to be able to change the type from "unsigned long" into size_t
some day in the future, we need to have a way to print 64 bit variables
on a system that has "unsigned long" defined to be 32 bit, like Win64.

Upcast all those variables into uintmax_t before they are printed.
This is to prepare for a bigger change, when "unsigned long"
will be converted into size_t for variables which may be > 4Gib.

Signed-off-by: Torsten Bögershausen 
---

Changes since V1:
- fixed typos in the commit message, thanks to Eric  Sunshime for careful 
reading

Applying it on pu  gives 1 conflict from the index/repo changes,
Should be easy to fix.

archive-tar.c  |  2 +-
 builtin/cat-file.c |  4 ++--
 builtin/fast-export.c  |  2 +-
 builtin/index-pack.c   |  9 +
 builtin/ls-tree.c  |  2 +-
 builtin/pack-objects.c | 12 ++--
 diff.c |  2 +-
 fast-import.c  |  4 ++--
 http-push.c|  2 +-
 ref-filter.c   |  2 +-
 sha1-file.c|  6 +++---
 11 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 7a535cba24..a58e1a8ebf 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -202,7 +202,7 @@ static void prepare_header(struct archiver_args *args,
   unsigned int mode, unsigned long size)
 {
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
-   xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? 
size : 0);
+   xsnprintf(header->size, sizeof(header->size), "%011"PRIoMAX , 
S_ISREG(mode) ? (uintmax_t)size : (uintmax_t)0);
xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned 
long) args->time);
 
xsnprintf(header->uid, sizeof(header->uid), "%07o", 0);
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8d97c84725..05decee33f 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -92,7 +92,7 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
oi.sizep = 
if (oid_object_info_extended(the_repository, , , flags) 
< 0)
die("git cat-file: could not get object info");
-   printf("%lu\n", size);
+   printf("%"PRIuMAX"\n", (uintmax_t)size);
return 0;
 
case 'e':
@@ -238,7 +238,7 @@ static void expand_atom(struct strbuf *sb, const char 
*atom, int len,
if (data->mark_query)
data->info.sizep = >size;
else
-   strbuf_addf(sb, "%lu", data->size);
+   strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size);
} else if (is_atom("objectsize:disk", atom, len)) {
if (data->mark_query)
data->info.disk_sizep = >disk_size;
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 456797c12a..5790f0d554 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -253,7 +253,7 @@ static void export_blob(const struct object_id *oid)
 
mark_next_object(object);
 
-   printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
+   printf("blob\nmark :%"PRIu32"\ndata %"PRIuMAX"\n", last_idnum, 
(uintmax_t)size);
if (size && fwrite(buf, size, 1, stdout) != 1)
die_errno("could not write blob '%s'", oid_to_hex(oid));
printf("\n");
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2004e25da2..2a8ada432b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -450,7 +450,8 @@ static void *unpack_entry_data(off_t offset, unsigned long 
size,
int hdrlen;
 
if (!is_delta_type(type)) {
-   hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", type_name(type), 
size) + 1;
+   hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX,
+  type_name(type),(uintmax_t)size) + 1;
the_hash_algo->init_fn();
the_hash_algo->update_fn(, hdr, hdrlen);
} else
@@ -1628,10 +1629,10 @@ static void show_pack_info(int stat_only)
chain_histogram[obj_stat[i].delta_depth - 1]++;
if (stat_only)
continue;
-   printf("%s %-6s %lu %lu %"PRIuMAX,
+   printf("%s %-6s %"PRIuMAX" %"PRIuMAX" %"PRIuMAX,
   oid_to_hex(>idx.oid),
-  type_name(obj->real_type), obj->size,
-  (unsigned long)(obj[1].idx.offset - obj->idx.offset),
+  type_name(obj->real_type), (uintmax_t)obj->size,
+  (uintmax_t)(obj[1].idx.offset - obj->idx.offset),
   (uintmax_t)obj->idx.offset);
if (is_delta_type(obj->type)) {
struct object_entry *bobj = 
[obj_stat[i].base_object_no];