git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/6] pass st.st_size as hint for strbuf_readlink()

From
Jeff King <peff@peff.net>
Date
Jul 26, 2018, 06:09 UTC
Message-ID
<20180726060951.GA25386@sigill.intra.peff.net>
In-Reply-To
<20180725184100.GA30961@tor.lan>
On Wed, Jul 25, 2018 at 08:41:00PM +0200, Torsten Bögershausen wrote:
Show 30 quoted lines
> 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"

I actually don't mind using xsize_t(), but if we're going into I think we should do it consistently. I.e., as a patch on top with that explanation.

-Peff
Previous: Torsten BögershausenNext: Jeff King
Message 8 of 9 in “use size_t in iconv/strbuf”
  1. 0/6 use size_t in iconv/strbufJeff King, Jul 24, 2018
  2. 1/6 reencode_string: use st_add/st_mult helpersJeff King, Jul 24, 2018
  3. 2/6 reencode_string: use size_t for string lengthsJeff King, Jul 24, 2018
  4. 3/6 strbuf: use size_t for length in intermediate variablesJeff King, Jul 24, 2018
  5. 4/6 strbuf_readlink: use ssize_tJeff King, Jul 24, 2018
  6. 5/6 pass st.st_size as hint for strbuf_readlink()Jeff King, Jul 24, 2018
  7. Torsten BögershausenJul 25, 2018
  8. Jeff KingJul 26, 2018
  9. 6/6 strbuf_humanise: use unsigned variablesJeff King, Jul 24, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.