[PATCH] http: fix some printf format warnings on 32-bit builds

[PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

Commit f8117f55 ("http: use off_t to store partial file size",
02-11-2015) changed the type of some variables from long to off_t.
The 32-bit build, which enables the large filesystem interface
(_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
integer, whereas long is a 32-bit integer. This results in a couple
of printf format warnings.

In order to suppress the warnings, change the format specifier to use
the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
the http_opt_request_remainder() function, which uses the same
solution).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Jeff,

I don't compile on 32-bit as often as I did in the past, otherwise I
would have noticed this sooner. :(

ATB,
Ramsay Jones

 http.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index 42f29ce..2532976 100644
--- a/http.c
+++ b/http.c
@@ -1617,8 +1617,8 @@ struct http_pack_request *new_http_pack_request(
 	if (prev_posn>0) {
 		if (http_is_verbose)
 			fprintf(stderr,
-				"Resuming fetch of pack %s at byte %ld\n",
-				sha1_to_hex(target->sha1), prev_posn);
+				"Resuming fetch of pack %s at byte %"PRIuMAX"\n",
+				sha1_to_hex(target->sha1), (uintmax_t)prev_posn);
 		http_opt_request_remainder(preq->slot->curl, prev_posn);
 	}
 
@@ -1772,8 +1772,8 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	if (prev_posn>0) {
 		if (http_is_verbose)
 			fprintf(stderr,
-				"Resuming fetch of object %s at byte %ld\n",
-				hex, prev_posn);
+				"Resuming fetch of object %s at byte %"PRIuMAX"\n",
+				hex, (uintmax_t)prev_posn);
 		http_opt_request_remainder(freq->slot->curl, prev_posn);
 	}
 
-- 
2.6.0

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Eric Sunshine]

On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
> Commit f8117f55 ("http: use off_t to store partial file size",
> 02-11-2015) changed the type of some variables from long to off_t.
> The 32-bit build, which enables the large filesystem interface
> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
> integer, whereas long is a 32-bit integer. This results in a couple
> of printf format warnings.
>
> In order to suppress the warnings, change the format specifier to use
> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
> the http_opt_request_remainder() function, which uses the same
> solution).

I just ran across the problem when building 'next' on my Mac and was
about to investigate, so am happy to find that the work has already
been done. Thanks.

My machine is 64-bit, though, so perhaps it's misleading to
characterize this as a fix for 32-bit builds. In particular, off_t is
'long long' on this machine, so it complains about the "long" format
specifier.

> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> diff --git a/http.c b/http.c
> index 42f29ce..2532976 100644
> --- a/http.c
> +++ b/http.c
> @@ -1617,8 +1617,8 @@ struct http_pack_request *new_http_pack_request(
>         if (prev_posn>0) {
>                 if (http_is_verbose)
>                         fprintf(stderr,
> -                               "Resuming fetch of pack %s at byte %ld\n",
> -                               sha1_to_hex(target->sha1), prev_posn);
> +                               "Resuming fetch of pack %s at byte %"PRIuMAX"\n",
> +                               sha1_to_hex(target->sha1), (uintmax_t)prev_posn);
>                 http_opt_request_remainder(preq->slot->curl, prev_posn);
>         }
>
> @@ -1772,8 +1772,8 @@ struct http_object_request *new_http_object_request(const char *base_url,
>         if (prev_posn>0) {
>                 if (http_is_verbose)
>                         fprintf(stderr,
> -                               "Resuming fetch of object %s at byte %ld\n",
> -                               hex, prev_posn);
> +                               "Resuming fetch of object %s at byte %"PRIuMAX"\n",
> +                               hex, (uintmax_t)prev_posn);
>                 http_opt_request_remainder(freq->slot->curl, prev_posn);
>         }
>
> --
> 2.6.0

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Stefan Beller]

On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>> Commit f8117f55 ("http: use off_t to store partial file size",
>> 02-11-2015) changed the type of some variables from long to off_t.
>> The 32-bit build, which enables the large filesystem interface
>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>> integer, whereas long is a 32-bit integer. This results in a couple
>> of printf format warnings.
>>
>> In order to suppress the warnings, change the format specifier to use
>> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
>> the http_opt_request_remainder() function, which uses the same
>> solution).
>
> I just ran across the problem when building 'next' on my Mac and was
> about to investigate, so am happy to find that the work has already
> been done. Thanks.
>
> My machine is 64-bit, though, so perhaps it's misleading to
> characterize this as a fix for 32-bit builds. In particular, off_t is
> 'long long' on this machine, so it complains about the "long" format
> specifier.

+Lars

I wonder if 32 bit compilation can be part of travis.

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Lars Schneider]

On 11 Nov 2015, at 03:00, Stefan Beller <sbeller@google.com> wrote:

> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>> <ramsay@ramsayjones.plus.com> wrote:
>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>> 02-11-2015) changed the type of some variables from long to off_t.
>>> The 32-bit build, which enables the large filesystem interface
>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>> integer, whereas long is a 32-bit integer. This results in a couple
>>> of printf format warnings.
>>> 
>>> In order to suppress the warnings, change the format specifier to use
>>> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
>>> the http_opt_request_remainder() function, which uses the same
>>> solution).
>> 
>> I just ran across the problem when building 'next' on my Mac and was
>> about to investigate, so am happy to find that the work has already
>> been done. Thanks.
>> 
>> My machine is 64-bit, though, so perhaps it's misleading to
>> characterize this as a fix for 32-bit builds. In particular, off_t is
>> 'long long' on this machine, so it complains about the "long" format
>> specifier.
> 
> +Lars
> 
> I wonder if 32 bit compilation can be part of travis.

Unfortunately no. All their machines are 64-bit [1] and they have "no immediate plans to add this feature" [2].
However, we could the following build configuration on a 64-bit machine:

export CFLAGS="-m32"
export LDFLAGS="-m32"

I think then we could catch these kind of warnings. Do you see a downside with this approach?

- Lars

[1] http://docs.travis-ci.com/user/ci-environment/
[2] https://github.com/travis-ci/travis-ci/issues/986

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Stefan Beller]

On Wed, Nov 11, 2015 at 12:02 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
> Unfortunately no. All their machines are 64-bit [1] and they have "no immediate plans to add this feature" [2].
> However, we could the following build configuration on a 64-bit machine:
>
> export CFLAGS="-m32"
> export LDFLAGS="-m32"
>
> I think then we could catch these kind of warnings. Do you see a downside with this approach?

I think that should do it.

Assuming their 64 bit means x86-64 architecture, we may want to prefer
-mx32, such that we can also test it?

>
> - Lars
>
> [1] http://docs.travis-ci.com/user/ci-environment/
> [2] https://github.com/travis-ci/travis-ci/issues/986

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

On 11/11/15 01:22, Eric Sunshine wrote:
> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>> Commit f8117f55 ("http: use off_t to store partial file size",
>> 02-11-2015) changed the type of some variables from long to off_t.
>> The 32-bit build, which enables the large filesystem interface
>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>> integer, whereas long is a 32-bit integer. This results in a couple
>> of printf format warnings.
>>
>> In order to suppress the warnings, change the format specifier to use
>> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
>> the http_opt_request_remainder() function, which uses the same
>> solution).
> 
> I just ran across the problem when building 'next' on my Mac and was
> about to investigate, so am happy to find that the work has already
> been done. Thanks.

Hmm, interesting. I've never used a Mac, so please forgive my ignorance ...

> My machine is 64-bit, though, so perhaps it's misleading to
> characterize this as a fix for 32-bit builds. In particular, off_t is
> 'long long' on this machine, so it complains about the "long" format
> specifier.

... but this seems to imply that sizeof(long) is 4 on your machine, right?
(on x86_64 linux it's 8, which is why I hadn't noticed before).

Jeff, do you need me to re-word the commit message?

ATB,
Ramsay Jones

> 
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>> diff --git a/http.c b/http.c
>> index 42f29ce..2532976 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -1617,8 +1617,8 @@ struct http_pack_request *new_http_pack_request(
>>         if (prev_posn>0) {
>>                 if (http_is_verbose)
>>                         fprintf(stderr,
>> -                               "Resuming fetch of pack %s at byte %ld\n",
>> -                               sha1_to_hex(target->sha1), prev_posn);
>> +                               "Resuming fetch of pack %s at byte %"PRIuMAX"\n",
>> +                               sha1_to_hex(target->sha1), (uintmax_t)prev_posn);
>>                 http_opt_request_remainder(preq->slot->curl, prev_posn);
>>         }
>>
>> @@ -1772,8 +1772,8 @@ struct http_object_request *new_http_object_request(const char *base_url,
>>         if (prev_posn>0) {
>>                 if (http_is_verbose)
>>                         fprintf(stderr,
>> -                               "Resuming fetch of object %s at byte %ld\n",
>> -                               hex, prev_posn);
>> +                               "Resuming fetch of object %s at byte %"PRIuMAX"\n",
>> +                               hex, (uintmax_t)prev_posn);
>>                 http_opt_request_remainder(freq->slot->curl, prev_posn);
>>         }
>>
>> --
>> 2.6.0
> 

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

On 11/11/15 02:00, Stefan Beller wrote:
> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>> <ramsay@ramsayjones.plus.com> wrote:
>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>> 02-11-2015) changed the type of some variables from long to off_t.
>>> The 32-bit build, which enables the large filesystem interface
>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>> integer, whereas long is a 32-bit integer. This results in a couple
>>> of printf format warnings.
>>>
>>> In order to suppress the warnings, change the format specifier to use
>>> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
>>> the http_opt_request_remainder() function, which uses the same
>>> solution).
>>
>> I just ran across the problem when building 'next' on my Mac and was
>> about to investigate, so am happy to find that the work has already
>> been done. Thanks.
>>
>> My machine is 64-bit, though, so perhaps it's misleading to
>> characterize this as a fix for 32-bit builds. In particular, off_t is
>> 'long long' on this machine, so it complains about the "long" format
>> specifier.
> 
> +Lars
> 
> I wonder if 32 bit compilation can be part of travis.
> 

Did this warning show up on the OS X build?

ATB,
Ramsay Jones

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Eric Sunshine]

On Wed, Nov 11, 2015 at 12:47 PM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
> On 11/11/15 01:22, Eric Sunshine wrote:
>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>> <ramsay@ramsayjones.plus.com> wrote:
>> My machine is 64-bit, though, so perhaps it's misleading to
>> characterize this as a fix for 32-bit builds. In particular, off_t is
>> 'long long' on this machine, so it complains about the "long" format
>> specifier.
>
> ... but this seems to imply that sizeof(long) is 4 on your machine, right?
> (on x86_64 linux it's 8, which is why I hadn't noticed before).

This code on my Mac:

    printf("sizeof(long)=%zu\n", sizeof(long));
    printf("sizeof(long long)=%zu\n", sizeof(long long));
    printf("sizeof(off_t)=%zu\n", sizeof(off_t));

produces:

    sizeof(long)=8
    sizeof(long long)=8
    sizeof(off_t)=8

The fact that 'long' and 'long long' happen to be the same size (in
this case) is immaterial. What is important is that the code is just
wrong to be using the "%l" specifier for 'long' when the actual
datatype is 'long long' (which is what 'off_t' is under-the-hood in
this case).

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Jeff King]

On Wed, Nov 11, 2015 at 03:31:01PM -0500, Eric Sunshine wrote:

> The fact that 'long' and 'long long' happen to be the same size (in
> this case) is immaterial. What is important is that the code is just
> wrong to be using the "%l" specifier for 'long' when the actual
> datatype is 'long long' (which is what 'off_t' is under-the-hood in
> this case).

Right. We cannot assume anything about what is in off_t, and should be
casting to uintmax_t. So the patch is right, but I agree the commit
message could be better. I started to hack it up myself, but I didn't
want to put too many words in Ramsay's mouth. Do you mind resending with
an updated commit message?

Thanks (and thank you in the first place for finding and fixing the
breakage I introduced in f8117f55).

-Peff

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

On 11/11/15 20:31, Eric Sunshine wrote:
> On Wed, Nov 11, 2015 at 12:47 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>> On 11/11/15 01:22, Eric Sunshine wrote:
>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>> <ramsay@ramsayjones.plus.com> wrote:
>>> My machine is 64-bit, though, so perhaps it's misleading to
>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>> 'long long' on this machine, so it complains about the "long" format
>>> specifier.
>>
>> ... but this seems to imply that sizeof(long) is 4 on your machine, right?
>> (on x86_64 linux it's 8, which is why I hadn't noticed before).
> 
> This code on my Mac:
> 
>     printf("sizeof(long)=%zu\n", sizeof(long));
>     printf("sizeof(long long)=%zu\n", sizeof(long long));
>     printf("sizeof(off_t)=%zu\n", sizeof(off_t));
> 
> produces:
> 
>     sizeof(long)=8
>     sizeof(long long)=8
>     sizeof(off_t)=8
> 
> The fact that 'long' and 'long long' happen to be the same size (in
> this case) is immaterial. What is important is that the code is just
> wrong to be using the "%l" specifier for 'long' when the actual
> datatype is 'long long' (which is what 'off_t' is under-the-hood in
> this case).

Ah. OK, so %ld for long and %lld for long long, I suppose.

Hmm, not that it matters, but I wonder what the PRId64 macro is. ;-)

ATB,
Ramsay Jones

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Torsten Bögershausen]

 >Ah. OK, so %ld for long and %lld for long long, I suppose. Only if you 
have a system that's support it.

Linux does, Windows not.

>Hmm, not that it matters, but I wonder what the PRId64 macro is. ;-)
It's "I64d" for Windows, and "lld" for all Gnu based systems and others,

When you do printf("%lld %ld", long_long_var, long_var),
the "printf runtime" under Windows will treat "%lld" as "%ld", and print
the lower part of long_long_var.
And will not pull a long long from stack, but a long, resulting i all kinds of confusion

So whenever a long long is printed, I can warmly recommend to use

PRId64

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

On 12/11/15 05:27, Torsten Bögershausen wrote:
>>Ah. OK, so %ld for long and %lld for long long, I suppose.
> Only if you have a system that's support it.
> 
> Linux does, Windows not.

Sure, but I was speculating specifically about Eric's mac (which I
have no experience with).

> 
>> Hmm, not that it matters, but I wonder what the PRId64 macro is. ;-)
> It's "I64d" for Windows, and "lld" for all Gnu based systems and others,

[On Gnu systems, I believe it is %lld on 32-bit and %ld on 64-bit.]

Again, I was commenting on Eric's mac, which _seems_ to allow
the use of both %ld and %lld when printing 64-bit integers, so
which does it choose for PRId64 ...

> 
> When you do printf("%lld %ld", long_long_var, long_var),
> the "printf runtime" under Windows will treat "%lld" as "%ld", and print
> the lower part of long_long_var.
> And will not pull a long long from stack, but a long, resulting i all kinds of confusion
> 
> So whenever a long long is printed, I can warmly recommend to use
> 
> PRId64

Indeed.

ATB,
Ramsay Jones

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Andreas Schwab]

Torsten Bögershausen <tboegi@web.de> writes:

> So whenever a long long is printed, I can warmly recommend to use
>
> PRId64

PRId64 is not suitable for long long, only for int64_t.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Lars Schneider]

On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

> 
> 
> On 11/11/15 02:00, Stefan Beller wrote:
>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>> <ramsay@ramsayjones.plus.com> wrote:
>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>> The 32-bit build, which enables the large filesystem interface
>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>> of printf format warnings.
>>>> 
>>>> In order to suppress the warnings, change the format specifier to use
>>>> the PRIuMAX macro and cast the off_t argument to uintmax_t. (See also
>>>> the http_opt_request_remainder() function, which uses the same
>>>> solution).
>>> 
>>> I just ran across the problem when building 'next' on my Mac and was
>>> about to investigate, so am happy to find that the work has already
>>> been done. Thanks.
>>> 
>>> My machine is 64-bit, though, so perhaps it's misleading to
>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>> 'long long' on this machine, so it complains about the "long" format
>>> specifier.
>> 
>> +Lars
>> 
>> I wonder if 32 bit compilation can be part of travis.
>> 
> 
> Did this warning show up on the OS X build?

Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI build and it breaks the build on OS X.
See here (you need to scroll all the way down):
https://travis-ci.org/larsxschneider/git/jobs/90899656

BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]

Is this a known issue? Is this an issue at all?

Thanks,
Lars

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Eric Sunshine]

On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
> On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>> On 11/11/15 02:00, Stefan Beller wrote:
>>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>>> <ramsay@ramsayjones.plus.com> wrote:
>>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>>> The 32-bit build, which enables the large filesystem interface
>>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>>> of printf format warnings.
>>>>
>>>> My machine is 64-bit, though, so perhaps it's misleading to
>>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>>> 'long long' on this machine, so it complains about the "long" format
>>>> specifier.
>>>
>>> I wonder if 32 bit compilation can be part of travis.
>>
>> Did this warning show up on the OS X build?
>
> Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI
> build and it breaks the build on OS X.
> See here (you need to scroll all the way down):
> https://travis-ci.org/larsxschneider/git/jobs/90899656
>
> BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
> ./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]
>
> Is this a known issue? Is this an issue at all?

Odd. I don't experience anything like that on my Mac.

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Torsten Bögershausen]

On 2015-11-13 09.57, Eric Sunshine wrote:
> On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
>> On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>>> On 11/11/15 02:00, Stefan Beller wrote:
>>>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>>>> <ramsay@ramsayjones.plus.com> wrote:
>>>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>>>> The 32-bit build, which enables the large filesystem interface
>>>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>>>> of printf format warnings.
>>>>>
>>>>> My machine is 64-bit, though, so perhaps it's misleading to
>>>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>>>> 'long long' on this machine, so it complains about the "long" format
>>>>> specifier.
>>>>
>>>> I wonder if 32 bit compilation can be part of travis.
>>>
>>> Did this warning show up on the OS X build?
>>
>> Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI
>> build and it breaks the build on OS X.
>> See here (you need to scroll all the way down):
>> https://travis-ci.org/larsxschneider/git/jobs/90899656
>>
>> BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
>> ./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]
>>
>> Is this a known issue? Is this an issue at all?
> 
> Odd. I don't experience anything like that on my Mac.

Could it be, that strlcpy is present on your system ?
And where does it come from ?

Which OS ?
Which compiler ?
What does `uname -r` say ?
Do you have Macports, Fink, Brew... installed ?

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Lars Schneider]

> On 13 Nov 2015, at 11:32, Torsten Bögershausen <tboegi@web.de> wrote:
> 
> On 2015-11-13 09.57, Eric Sunshine wrote:
>> On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider
>> <larsxschneider@gmail.com> wrote:
>>> On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>>>> On 11/11/15 02:00, Stefan Beller wrote:
>>>>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>>>>> <ramsay@ramsayjones.plus.com> wrote:
>>>>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>>>>> The 32-bit build, which enables the large filesystem interface
>>>>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>>>>> of printf format warnings.
>>>>>> 
>>>>>> My machine is 64-bit, though, so perhaps it's misleading to
>>>>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>>>>> 'long long' on this machine, so it complains about the "long" format
>>>>>> specifier.
>>>>> 
>>>>> I wonder if 32 bit compilation can be part of travis.
>>>> 
>>>> Did this warning show up on the OS X build?
>>> 
>>> Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI
>>> build and it breaks the build on OS X.
>>> See here (you need to scroll all the way down):
>>> https://travis-ci.org/larsxschneider/git/jobs/90899656
>>> 
>>> BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
>>> ./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]
>>> 
>>> Is this a known issue? Is this an issue at all?
>> 
>> Odd. I don't experience anything like that on my Mac.
> 
> Could it be, that strlcpy is present on your system ?
> And where does it come from ?
> 
> Which OS ?
> Which compiler ?
> What does `uname -r` say ?
> Do you have Macports, Fink, Brew... installed ?
> 

Looks like this is a OS X only issue. Happens with clang and gcc on the OS X Mavericks TravisCI machines [1]:
https://travis-ci.org/larsxschneider/git/jobs/90919078
https://travis-ci.org/larsxschneider/git/jobs/90919080

On Linux+gcc the following error happens if "-Werror" is present:
https://travis-ci.org/larsxschneider/git/jobs/90919076
Do you have an idea what that might be?

Linux+clang works fine:
https://travis-ci.org/larsxschneider/git/jobs/90919074

- Lars

[1] http://docs.travis-ci.com/user/ci-environment/

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Ramsay Jones]

On 13/11/15 08:57, Eric Sunshine wrote:
> On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
>> On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>>> On 11/11/15 02:00, Stefan Beller wrote:
>>>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>>>> <ramsay@ramsayjones.plus.com> wrote:
>>>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>>>> The 32-bit build, which enables the large filesystem interface
>>>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>>>> of printf format warnings.
>>>>>
>>>>> My machine is 64-bit, though, so perhaps it's misleading to
>>>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>>>> 'long long' on this machine, so it complains about the "long" format
>>>>> specifier.
>>>>
>>>> I wonder if 32 bit compilation can be part of travis.
>>>
>>> Did this warning show up on the OS X build?
>>
>> Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI
>> build and it breaks the build on OS X.
>> See here (you need to scroll all the way down):
>> https://travis-ci.org/larsxschneider/git/jobs/90899656
>>
>> BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
>> ./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]
>>
>> Is this a known issue? Is this an issue at all?
> 
> Odd. I don't experience anything like that on my Mac.
> 

Hmm, from the output, it looks like the configure script is
not detecting that 'strlcpy' is available (so setting
NO_STRLCPY=YesPlease in the config.mak.autogen file).
However, it seems to be a 'macro redirect' set in the
/usr/include/secure/_string.h header file (presumably it
redirects between a more or less secure version ;-)

Unfortunately, I don't have access to a mac - so I can't
help you with the debugging. :(

ATB,
Ramsay Jones

Re: [PATCH] http: fix some printf format warnings on 32-bit builds

[Lars Schneider]

On 13 Nov 2015, at 21:02, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

> 
> 
> On 13/11/15 08:57, Eric Sunshine wrote:
>> On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider
>> <larsxschneider@gmail.com> wrote:
>>> On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>>>> On 11/11/15 02:00, Stefan Beller wrote:
>>>>> On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>>>> On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones
>>>>>> <ramsay@ramsayjones.plus.com> wrote:
>>>>>>> Commit f8117f55 ("http: use off_t to store partial file size",
>>>>>>> 02-11-2015) changed the type of some variables from long to off_t.
>>>>>>> The 32-bit build, which enables the large filesystem interface
>>>>>>> (_FILE_OFFSET_BITS == 64), defines the off_t type as a 64-bit
>>>>>>> integer, whereas long is a 32-bit integer. This results in a couple
>>>>>>> of printf format warnings.
>>>>>> 
>>>>>> My machine is 64-bit, though, so perhaps it's misleading to
>>>>>> characterize this as a fix for 32-bit builds. In particular, off_t is
>>>>>> 'long long' on this machine, so it complains about the "long" format
>>>>>> specifier.
>>>>> 
>>>>> I wonder if 32 bit compilation can be part of travis.
>>>> 
>>>> Did this warning show up on the OS X build?
>>> 
>>> Yes, I added CFLAGS="-Werror=format" to the my experimental TravisCI
>>> build and it breaks the build on OS X.
>>> See here (you need to scroll all the way down):
>>> https://travis-ci.org/larsxschneider/git/jobs/90899656
>>> 
>>> BTW: I tried to set "-Werror" but then I got a bunch of macro redefined errors like this:
>>> ./git-compat-util.h:614:9: error: 'strlcpy' macro redefined [-Werror]
>>> 
>>> Is this a known issue? Is this an issue at all?
>> 
>> Odd. I don't experience anything like that on my Mac.
>> 
> 
> Hmm, from the output, it looks like the configure script is
> not detecting that 'strlcpy' is available (so setting
> NO_STRLCPY=YesPlease in the config.mak.autogen file).
> However, it seems to be a 'macro redirect' set in the
> /usr/include/secure/_string.h header file (presumably it
> redirects between a more or less secure version ;-)
> 
> Unfortunately, I don't have access to a mac - so I can't
> help you with the debugging. :(
> 

I don't have any experience with autotools at all. However, here is what I found out on OS X Mavericks (10.9.5):
1.) In config.mak.uname, line 103, "NO_STRLCPY = YesPlease" is set for some old OS X version. However, it looks like these settings have no impact at all.
2.) In configure.ac, line 1010, the AC_CHECK_FUNC macros (via GIT_CHECK_FUNC) is used to detect strlcpy. That detection fails on OS X Mavericks.
3.) I tried to use "AC_CHECK_DECLS" to detect strlcpy declarations on OS X and this works.

Can you give me a few hints how to debug this further? Why do have the values in config.mak.uname no impact? My idea was to detect OS X Mavericks there and unset NO_STRLCPY. Could that work?

Thanks,
Lars