[Privoxy-devel] warning: unknown conversion type character 'l' in format

[Lee]

On 6/7/17, Fabian Keil <fk at fabiankeil.de> wrote:
> Lee <ler762 at gmail.com> wrote:
>
>> GCC fusses about an %llu format and seems to translate it into %lu
>
> What do you mean by "translate"?

Rather than crashing or otherwise misbehaving, the program acts like
it was given a format of "%lu"

> Does the %llu not end up in the binary?

I can grep for "%llu" in parsers.o & get "Binary file parsers.o
matches" but I don't know how to tell if that specific "%llu" ends up
in the binary or not :(

>> Other than the compiler warning, I haven't noticed any problems with
>> %llu but it would be nice to get rid of another compiler warning
>> message.
>>
>> in parsers.c add another #ifdef _WIN32 section to use %I64u instead of
>> %llu:
>>
>> static void create_content_length_header(unsigned long long
>> content_length,
>>                                          char *header, size_t
>> buffer_length)
>> {
>> #ifdef _WIN32
>>    assert(sizeof(unsigned long long) > 4);
>>    snprintf(header, buffer_length, "Content-Length: %I64u",
>> content_length);
>>    log_error(LOG_LEVEL_INFO, "create_content_length_header:
>> Content-Length: %llu", content_length);
> [...]
>> ------- with %I64u
>> 2017-06-05 14:05:40.360 00000d9c Request: www.installsite.org/
>> 2017-06-05 14:05:40.578 00000d9c Info: buffer_and_filter_content set to 1
>> 2017-06-05 14:05:40.578 00000d9c Info: create_content_length_header:
>> Content-Length: 9855
>
> If there was a problem with the %I64u it would be unlikely to
> affect the log message as it doesn't print the "header" string.
>
> Errors are probably also more likely to show up with values
> that can't be represented with 32 bit.

Can you share a good reference for how C does type casting?

Some of the WIN32 specific things like write_socket in jbsockets.c
where it casts size_t len to (int)len for the send call seems like an
error waiting to happen.  Unless len is somehow guaranteed never to be
greater than 2^31-1??  ( size_t is unsigned, (int) is signed, so it
seems to me that as long as the value of len fits in 31 bits
everything will be fine.  What happens when the value of len is
greater than 2^31?)

>> Can anyone think of a reason _not_ to add the
>>
>> #ifdef _WIN32
>>    assert(sizeof(unsigned long long) > 4);
>>    snprintf(header, buffer_length, "Content-Length: %I64u",
>> content_length);
>> #else
>>
>> bit to parsers.c::create_content_length_header ?
>
> I have no objections to the snprint() with the modified
> format specifier but one reason not to add the assertion
> is that the condition can be checked at compile time.
>
> Probably we should replace the assertion I added in r1.209
> with something like:
>
> commit bea35441b9f8e2bf78109544ebc54a17b8373cdd
> Author: Fabian Keil <fk at fabiankeil.de>
> Date:   Wed Jun 7 12:57:44 2017 +0200
>
>     get_content_length(): Replace an assertion with a compile-time check
>
> diff --git a/parsers.c b/parsers.c
> index 97528dba..011641d7 100644
> --- a/parsers.c
> +++ b/parsers.c
> @@ -1816,7 +1816,9 @@ static jb_err client_keep_alive(struct client_state
> *csp, char **header)
>  static jb_err get_content_length(const char *header_value, unsigned long
> long *length)
>  {
>  #ifdef _WIN32
> -   assert(sizeof(unsigned long long) > 4);
> +#if SIZEOF_LONG_LONG < 8
> +#error sizeof(unsigned long long) too small
> +#endif
>     if (1 != sscanf(header_value, "%I64u", length))
>  #else
>     if (1 != sscanf(header_value, "%llu", length))
>
> instead of adding more of them.

I don't know enough to have an informed opinion, so I'll go with
whatever you think is best.

& just to be clear - you're ok with me applying this patch?

Index: current/parsers.c
===================================================================
RCS file: /cvsroot/ijbswa/current/parsers.c,v
retrieving revision 1.311
diff -U5 -d -r1.311 parsers.c
--- current/parsers.c   24 Dec 2016 16:00:49 -0000      1.311
+++ current/parsers.c   7 Jun 2017 18:24:02 -0000
@@ -4642,11 +4642,18 @@
  *
  *********************************************************************/
 static void create_content_length_header(unsigned long long content_length,
                                          char *header, size_t buffer_length)
 {
+#ifdef _WIN32
+#if SIZEOF_LONG_LONG < 8
+#error sizeof(unsigned long long) too small
+#endif
+   snprintf(header, buffer_length, "Content-Length: %I64u", content_length);
+#else
    snprintf(header, buffer_length, "Content-Length: %llu", content_length);
+#endif
 }


Thanks,
Lee