git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, antoine.queru@ensimag.grenoble-inp.fr,
	francois.beutin@ensimag.grenoble-inp.fr, mhagger@alum.mit.edu,
	Johannes.Schindelin@gmx.de, mh@glandium.org
Subject: Re: [PATCH V2 3/3] strbuf: allow to use preallocated memory
Date: Tue, 07 Jun 2016 11:10:14 -0700	[thread overview]
Message-ID: <xmqqziqwq395.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: 20160607090653.GA4665@Messiaen

William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:

>> Perhaps I made it clearer by using a more exaggerated example e.g. a
>> typical expected buffer size of 128 bytes, but the third line of
>> 1000 line input file was an anomaly that is 200k bytes long.  I do
>> not want to keep that 200k bytes after finishing to process that
>> third line while using its initial 80 bytes for the remaining 977
>> lines.
>
> "its initial 128 bytes", rather than "its initial 80 bytes", no?

Either way would work, but 80 is closer to what I had in mind, as
the set-up of the example is "I know 99% of my input will fit within
80, but I'll allocate 128 to avoid realloc cost when there are rare
ones that bust 80.  I do not want to die only because there is an
occasional oddball that needs 200".

> The "fixed" feature was aimed to allow the users to use strbuf with
> strings that they doesn't own themselves (a function parameter for
> example). From Michael example in the original mail:
>
> void f(char *path_buf, size_t path_buf_len)
> {
>     struct strbuf path;
>     strbuf_wrap_fixed(&path, path_buf,
>     strlen(path_buf),
>     path_buf_len);
>     ...
>     /*
>      * no strbuf_release() required here, but if called it
>      * is a NOOP
>      */
> }

Think what does the "..." part would do using the "path" strbuf.

If 'f()' is meant to take the "dying is perfectly fine if the data
we have to process exceeds the area we were given even by one byte"
attitude, then the "capped to the same length as allocated" is
perfectly fine, but if 'f()' cannot afford to die() and instead has
to signal an error condition to its caller, then this function has
to check the length currently in use (i.e. path.len) and how much
more memory it can still use, before making each call to strbuf_*()
functions, no?

If we were to add "fixed" feature, we'd want to see it to help a
function like f() that cannot afford to die() and does not want to
malloc()/realloc().  I do not think what was in this series was it.

  reply	other threads:[~2016-06-07 18:10 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-06 15:13 [PATCH V2 0/3] strbuf: improve API William Duclot
2016-06-06 15:13 ` [PATCH V2 1/3] strbuf: add tests William Duclot
2016-06-06 16:11   ` Matthieu Moy
2016-06-07  8:44   ` Johannes Schindelin
2016-06-06 15:13 ` [PATCH V2 2/3] pretty.c: rename strbuf_wrap() function William Duclot
2016-06-06 16:12   ` Matthieu Moy
2016-06-07  9:04   ` Johannes Schindelin
2016-06-06 15:13 ` [PATCH V2 3/3] strbuf: allow to use preallocated memory William Duclot
2016-06-06 16:17   ` Matthieu Moy
2016-06-06 17:19   ` Junio C Hamano
2016-06-06 20:39     ` William Duclot
2016-06-06 22:44       ` Junio C Hamano
2016-06-06 22:58         ` Jeff King
2016-06-06 23:24           ` Junio C Hamano
2016-06-06 23:25             ` Junio C Hamano
2016-06-06 23:30             ` Jeff King
2016-06-07  9:06             ` William Duclot
2016-06-07 18:10               ` Junio C Hamano [this message]
2016-06-08 16:20               ` Michael Haggerty
2016-06-08 18:07                 ` Junio C Hamano
2016-06-08 19:19                 ` Jeff King
2016-06-08 19:42                   ` [PATCH] send-pack: use buffered I/O to talk to pack-objects Jeff King
2016-06-09 12:10                     ` Matthieu Moy
2016-06-09 14:34                       ` Ramsay Jones
2016-06-09 17:12                         ` Jeff King
2016-06-09 22:40                           ` Ramsay Jones
2016-06-09 16:40                       ` Junio C Hamano
2016-06-09 17:14                         ` Jeff King
2016-06-09 17:22                         ` Matthieu Moy
2016-06-08 19:48                   ` [PATCH V2 3/3] strbuf: allow to use preallocated memory Junio C Hamano
2016-06-08 19:52                     ` Jeff King
2016-06-08 23:05                       ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqziqwq395.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=antoine.queru@ensimag.grenoble-inp.fr \
    --cc=francois.beutin@ensimag.grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=mh@glandium.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    --cc=william.duclot@ensimag.grenoble-inp.fr \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).