From: "Torsten Bögershausen" <tboegi@web.de>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/6] pass st.st_size as hint for strbuf_readlink()
Date: Wed, 25 Jul 2018 20:41:00 +0200 [thread overview]
Message-ID: <20180725184100.GA30961@tor.lan> (raw)
In-Reply-To: <20180724105139.GE17165@sigill.intra.peff.net>
On Tue, Jul 24, 2018 at 06:51:39AM -0400, Jeff King wrote:
> When we initially added the strbuf_readlink() function in
> b11b7e13f4 (Add generic 'strbuf_readlink()' helper function,
> 2008-12-17), the point was that we generally have a _guess_
> as to the correct size based on the stat information, but we
> can't necessarily trust it.
>
> Over the years, a few callers have grown up that simply pass
> in 0, even though they have the stat information. Let's have
> them pass in their hint for consistency (and in theory
> efficiency, since it may avoid an extra resize/syscall loop,
> but neither location is probably performance critical).
>
> Note that st.st_size is actually an off_t, so in theory we
> need xsize_t() here. But none of the other callsites use it,
> and since this is just a hint, it doesn't matter either way
> (if we wrap we'll simply start with a too-small hint and
> then eventually complain when we cannot allocate the
> memory).
Thanks a lot for the series.
For the last paragraph I would actually vote the other way around -
how about something like this ?
Note that st.st_size is actually an off_t, so we should use
xsize_t() here. In pratise we don't expect links to be large like that,
but let's give a good example in the source code and use xsize_t()
whenever an off_t is converted into size_t.
This will make live easier whenever someones diggs into 32/64 bit
"conversion safetyness"
next prev parent reply other threads:[~2018-07-25 18:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-24 10:48 [PATCH 0/6] use size_t in iconv/strbuf Jeff King
2018-07-24 10:50 ` [PATCH 1/6] reencode_string: use st_add/st_mult helpers Jeff King
2018-07-24 10:50 ` [PATCH 2/6] reencode_string: use size_t for string lengths Jeff King
2018-07-24 10:51 ` [PATCH 3/6] strbuf: use size_t for length in intermediate variables Jeff King
2018-07-24 10:51 ` [PATCH 4/6] strbuf_readlink: use ssize_t Jeff King
2018-07-24 10:51 ` [PATCH 5/6] pass st.st_size as hint for strbuf_readlink() Jeff King
2018-07-25 18:41 ` Torsten Bögershausen [this message]
2018-07-26 6:09 ` Jeff King
2018-07-24 10:52 ` [PATCH 6/6] strbuf_humanise: use unsigned variables Jeff King
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=20180725184100.GA30961@tor.lan \
--to=tboegi@web.de \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/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.