All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tarmigan <tarmigan+git@gmail.com>
To: "Shawn O. Pearce" <spearce@spearce.org>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] http-backend: Fix bad treatment of uintmax_t in  Content-Length
Date: Sat, 14 Nov 2009 13:49:55 -0800	[thread overview]
Message-ID: <905315640911141349t7121baa8vc0b2be59fa348512@mail.gmail.com> (raw)
In-Reply-To: <20091112044240.GP11919@spearce.org>

On Wed, Nov 11, 2009 at 8:42 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Our Content-Length needs to report an off_t, which could be larger
> precision than size_t on this system (e.g. 32 bit binary built with
> 64 bit large file support).
>
> We also shouldn't be passing a size_t parameter to printf when
> we've used PRIuMAX as the format specifier.
>
> Fix both issues by using uintmax_t for the hdr_int() routine,
> allowing strbuf's size_t to automatically upcast, and off_t to
> always fit.
>
> Also fixed the copy loop we use inside of send_local_file(), we never
> actually updated the size variable so we might as well not use it.
>
> Reported-by: Tarmigan <tarmigan+git@gmail.com>

Tested-by: Tarmigan <tarmigan+git@gmail.com>

> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>
>  Tarmigan <tarmigan+git@gmail.com> wrote:
>  > unhappy.  Curl returns 18 (CURLE_PARTIAL_FILE), the test takes a long
>  > time to fail, and the "out" file looks OK (compared to a linux machine
>  > where the test passes) expect for "Content-Length: 37847251812411".
>  >
>  > Digging into it a bit more with gdb, the call to hdr_int() in
>  > http-backend.c looks OK, but then something goes wrong in
>  > format_write().  Hmmm it looks like my setup does not like PRIuMAX
>  > with size_t, which puts some garbage in the upper bytes of
>
>  Yup, only the right fix is to keep using PRIuMAX... patch below.

This fix is better than the (uintmax_t) cast that I was thinking about posting.

Please also consider the "__attribute__((format(printf,1,2))" patches
that I just posted to the list that would warn about this in the
future.

Thanks,
Tarmigan

  reply	other threads:[~2009-11-14 21:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-09  5:18 What's cooking in git.git (Nov 2009, #02; Sun, 08) Junio C Hamano
2009-11-09  8:08 ` Tarmigan
2009-11-09 15:29   ` Shawn O. Pearce
2009-11-10 17:10 ` Tarmigan
2009-11-12  4:42   ` [PATCH] http-backend: Fix bad treatment of uintmax_t in Content-Length Shawn O. Pearce
2009-11-14 21:49     ` Tarmigan [this message]
2009-11-15  9:08       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=905315640911141349t7121baa8vc0b2be59fa348512@mail.gmail.com \
    --to=tarmigan+git@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.