All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: git <git@vger.kernel.org>
Subject: Re: Project idea: strbuf allocation modes
Date: Tue, 22 Apr 2014 09:07:16 +0200	[thread overview]
Message-ID: <vpqr44psuzf.fsf@anie.imag.fr> (raw)
In-Reply-To: <53512DB6.1070600@alum.mit.edu> (Michael Haggerty's message of "Fri, 18 Apr 2014 15:50:46 +0200")

Michael Haggerty <mhagger@alum.mit.edu> writes:

>     STRBUF_OWNS_MEMORY
>
>         The memory pointed to by buf is owned by this strbuf.  If this
>         bit is not set, then the memory should never be freed, and
>         (among other things) strbuf_detach() must always call xstrcpy().

I just foresee a small difficulty from the caller side:

            char path_buf[PATH_MAX];
            char *detached;

            struct strbuf path;

            strbuf_wrap_preallocated(&path, path_buf, 0, sizeof(path));
            ...
            detached = strbuf_detach(&path);

the wrapping/unwrapping of path_buf magically turned a stack-allocated
buffer into a heap-allocated one. And the initial goal of avoiding
malloc() is defeated. So, essentially, one should avoir using
strbuf_wrap_preallocated with strbuf_detach, right? But I agree with
Junio that if the API is properly used, everything should work. I'm just
worried that we will add a bit more complexity to the API, and I'm not
sure we can actually expect noticeable improvements in terms of
performance (i.e. do we actually use strbuf for performance-critical
stuff?).

In your proposal, would STRBUF_OWNS_MEMORY be a constant, or a flag that
change when the internal buffer needs reallocation? My understanding is
that it should change (if STRBUF_FIXED_MEMORY is not set), and the
strbuf wrapping a preallocated buffer would become a "normal" strbuf
when its internal buffer grows.

If so, your "strbuf_detach() must always call xstrcpy()" is to be
understood as "if STRBUF_OWNS_MEMORY is still set when strbuf_detach()
is called, then it must always call xstrcpy()", right?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  parent reply	other threads:[~2014-04-22  7:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-08 15:44 Students project on Git (Ensimag) Matthieu Moy
2014-04-18 13:50 ` Project idea: strbuf allocation modes Michael Haggerty
2014-04-18 17:21   ` Junio C Hamano
2014-04-18 20:04     ` Michael Haggerty
2014-04-19  0:55       ` Junio C Hamano
2014-04-22  7:07   ` Matthieu Moy [this message]
2014-04-22  9:09     ` Michael Haggerty
2014-04-22 10:38       ` Matthieu Moy

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=vpqr44psuzf.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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.