threads / patch / 40766

patchhttp: fix some printf format warnings on 32-bit builds

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

## tl;dr

19 messages between Nov 11, 2015 and Nov 14, 2015. Diffs are folded; open one to read it.

replies: 18people: 7as markdown or json

Ramsay Jones· Nov 11, 2015, 00:23 UTC · lore

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(-)
Show changes to http.c +4 −4
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
Eric Sunshine· Nov 11, 2015, 01:22 UTC · re: Ramsay Jones · lore

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

On Tue, Nov 10, 2015 at 7:23 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 11 quoted lines
> 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.

Show 30 quoted lines
> 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
Stefan Beller· Nov 11, 2015, 02:00 UTC · re: Eric Sunshine · lore

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

On Tue, Nov 10, 2015 at 5:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 22 quoted lines
> 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.
Lars Schneider· Nov 11, 2015, 08:02 UTC · re: Stefan Beller · lore

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

On 11 Nov 2015, at 03:00, Stefan Beller <sbeller@google.com> wrote:
Show 27 quoted lines
> 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

Stefan Beller· Nov 11, 2015, 17:38 UTC · re: Lars Schneider · lore

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

On Wed, Nov 11, 2015 at 12:02 AM, Lars Schneider <larsxschneider@gmail.com> wrote:

Show 8 quoted lines
>
> 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?

Show 5 quoted lines
>
> - Lars
>
> [1] http://docs.travis-ci.com/user/ci-environment/
> [2] https://github.com/travis-ci/travis-ci/issues/986
Ramsay Jones· Nov 11, 2015, 17:49 UTC · re: Stefan Beller · lore

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

On 11/11/15 02:00, Stefan Beller wrote:
Show 28 quoted lines
> 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

Lars Schneider· Nov 13, 2015, 08:46 UTC · re: Ramsay Jones · lore

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

On 11 Nov 2015, at 18:49, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
Show 33 quoted lines
> 
> 
> 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

Eric Sunshine· Nov 13, 2015, 08:57 UTC · re: Lars Schneider · lore

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

On Fri, Nov 13, 2015 at 3:46 AM, Lars Schneider <larsxschneider@gmail.com> wrote:

Show 30 quoted lines
> 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.
Torsten Bögershausen· Nov 13, 2015, 10:32 UTC · re: Eric Sunshine · lore

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

On 2015-11-13 09.57, Eric Sunshine wrote:
Show 34 quoted lines
> 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 ?

Lars Schneider· Nov 13, 2015, 12:08 UTC · re: Torsten Bögershausen · lore

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

Show 46 quoted lines
> 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/
Ramsay Jones· Nov 13, 2015, 20:02 UTC · re: Eric Sunshine · lore

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

On 13/11/15 08:57, Eric Sunshine wrote:
Show 35 quoted lines
> 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

Lars Schneider· Nov 14, 2015, 19:14 UTC · re: Ramsay Jones · lore

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

On 13 Nov 2015, at 21:02, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
Show 49 quoted lines
> 
> 
> 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

Ramsay Jones· Nov 11, 2015, 17:47 UTC · re: Eric Sunshine · lore

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

On 11/11/15 01:22, Eric Sunshine wrote:
Show 17 quoted lines
> 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

Show 32 quoted lines
> 
>> 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
>
Eric Sunshine· Nov 11, 2015, 20:31 UTC · re: Ramsay Jones · lore

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

On Wed, Nov 11, 2015 at 12:47 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 10 quoted lines
> 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).

Jeff King· Nov 11, 2015, 20:54 UTC · re: Eric Sunshine · lore

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

On Wed, Nov 11, 2015 at 03:31:01PM -0500, Eric Sunshine wrote:
Show 5 quoted lines
> 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
Ramsay Jones· Nov 11, 2015, 20:58 UTC · re: Eric Sunshine · lore

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

On 11/11/15 20:31, Eric Sunshine wrote:
Show 30 quoted lines
> 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

Torsten Bögershausen· Nov 12, 2015, 05:27 UTC · re: Ramsay Jones · lore

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

 >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
Ramsay Jones· Nov 12, 2015, 11:58 UTC · re: Torsten Bögershausen · lore

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

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 ...

Show 9 quoted lines
> 
> 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

Andreas Schwab· Nov 12, 2015, 18:40 UTC · re: Torsten Bögershausen · lore

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

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."

← back to recent threads