Commit Graph

116 Commits

Author SHA1 Message Date
Junio C Hamano
a48ce37858 Merge branch 'ma/ts-cleanups'
Assorted bugfixes and clean-ups.

* ma/ts-cleanups:
  ThreadSanitizer: add suppressions
  strbuf_setlen: don't write to strbuf_slopbuf
  pack-objects: take lock before accessing `remaining`
  convert: always initialize attr_action in convert_attrs
2017-09-10 17:08:22 +09:00
Martin Ågren
65961d5a75 strbuf_setlen: don't write to strbuf_slopbuf
strbuf_setlen(., 0) writes '\0' to sb.buf[0], where buf is either
allocated and unique to sb, or the global slopbuf. The slopbuf is meant
to provide a guarantee that buf is not NULL and that a freshly
initialized buffer contains the empty string, but it is not supposed to
be written to. That strbuf_setlen writes to slopbuf has at least two
implications:

First, it's wrong in principle. Second, it might be hiding misuses which
are just waiting to wreak havoc. Third, ThreadSanitizer detects a race
when multiple threads write to slopbuf at roughly the same time, thus
potentially making any more critical races harder to spot.

Avoid writing to strbuf_slopbuf in strbuf_setlen. Let's instead assert
on the first byte of slopbuf being '\0', since it helps ensure the
promised invariant of buf[len] == '\0'. (We know that "len" was already
0, or someone has messed with "alloc". If someone has fiddled with the
fields that much beyond the correct interface, they're on their own.)

This is a function which is used in many places, possibly also in hot
code paths. There are two branches in strbuf_setlen already, and we are
adding a third and possibly a fourth (in the assert). In hot code paths,
we hopefully reuse the buffer in order to avoid continous reallocations.
Thus, after a start-up phase, we should always take the same path,
which might help branch prediction, and we would never make the assert.
If a hot code path continuously reallocates, we probably have bigger
performance problems than this new safety-check.

Simple measurements do not contradict this reasoning. 100000000 times
resetting a buffer and adding the empty string takes 5.29/5.26 seconds
with/without this patch (best of three). Releasing at every iteration
yields 18.01/17.87. Adding a 30-character string instead of the empty
string yields 5.61/5.58 and 17.28/17.28(!).

This patch causes the git binary emitted by gcc 5.4.0 -O2 on my machine
to grow from 11389848 bytes to 11497184 bytes, an increase of 0.9%.

I also tried to piggy-back on the fact that we already check alloc,
which should already tell us whether we are using the slopbuf:

        if (sb->alloc) {
                if (len > sb->alloc - 1)
                        die("BUG: strbuf_setlen() beyond buffer");
                sb->buf[len] = '\0';
        } else {
                if (len)
                        die("BUG: strbuf_setlen() beyond buffer");
                assert(!strbuf_slopbuf[0]);
        }
        sb->len = len;

That didn't seem to be much slower (5.38, 18.02, 5.70, 17.32 seconds),
but it does introduce some minor code duplication. The resulting git
binary was 11510528 bytes large (another 0.1% increase).

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-23 10:38:46 -07:00
Jeff King
cbc0f81d96 strbuf: use designated initializers in STRBUF_INIT
There are certain C99 features that might be nice to use in
our code base, but we've hesitated to do so in order to
avoid breaking compatibility with older compilers. But we
don't actually know if people are even using pre-C99
compilers these days.

One way to figure that out is to introduce a very small use
of a feature, and see if anybody complains. The strbuf code
is a good place to do this for a few reasons:

  - it always gets compiled, no matter which Makefile knobs
    have been tweaked.

  - it's very stable; this definition hasn't changed in a
    long time and is not likely to (so if we have to revert,
    it's unlikely to cause headaches)

If this patch can survive a few releases without complaint,
then we can feel more confident that designated initializers
are widely supported by our user base.  It also is an
indication that other C99 features may be supported, but not
a guarantee (e.g., gcc had designated initializers before
C99 existed).

And if we do get complaints, then we'll have gained some
data and we can easily revert this patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-14 08:32:44 -07:00
Junio C Hamano
6ba649e408 Merge branch 'ab/strbuf-addftime-tzname-boolify'
strbuf_addftime() is further getting tweaked.

* ab/strbuf-addftime-tzname-boolify:
  strbuf: change an always NULL/"" strbuf_addftime() param to bool
  strbuf.h comment: discuss strbuf_addftime() arguments in order
2017-07-06 18:14:47 -07:00
Ævar Arnfjörð Bjarmason
3b702239d6 strbuf: change an always NULL/"" strbuf_addftime() param to bool
strbuf_addftime() allows callers to pass a time zone name for
expanding %Z. The only current caller either passes the empty string
or NULL, in which case %Z is handed over verbatim to strftime(3).
Replace that string parameter with a flag controlling whether to
remove %Z from the format specification. This simplifies the code.

Commit-message-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-01 10:47:05 -07:00
Junio C Hamano
8af3c643d9 Merge branch 'rs/pretty-add-again'
The pretty-format specifiers like '%h', '%t', etc. had an
optimization that no longer works correctly.  In preparation/hope
of getting it correctly implemented, first discard the optimization
that is broken.

* rs/pretty-add-again:
  pretty: recalculate duplicate short hashes
2017-06-24 14:28:38 -07:00
Ævar Arnfjörð Bjarmason
4904cbc9e1 strbuf.h comment: discuss strbuf_addftime() arguments in order
Change the comment documenting the strbuf_addftime() function to
discuss the parameters in the order in which they appear, which makes
this easier to read than discussing them out of order.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-24 11:15:59 -07:00
René Scharfe
c3fbf81a85 strbuf: let strbuf_addftime handle %z and %Z itself
There is no portable way to pass timezone information to strftime.  Add
parameters for timezone offset and name to strbuf_addftime and let it
handle the timezone-related format specifiers %z and %Z internally.

Callers can opt out for %Z by passing NULL as timezone name.  %z is
always handled internally -- this helps on Windows, where strftime would
expand it to a timezone name (same as %Z), in violation of POSIX.
Modifiers are not handled, e.g. %Ez is still passed to strftime.

Use an empty string as timezone name in show_date (the only current
caller) for now because we only have the timezone offset in non-local
mode.  POSIX allows %Z to resolve to an empty string in case of missing
information.

Helped-by: Ulrich Mueller <ulm@gentoo.org>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 14:34:37 -07:00
René Scharfe
fe9e2aefd4 pretty: recalculate duplicate short hashes
b9c6232138 (--format=pretty: avoid calculating expensive expansions
twice) optimized adding short hashes multiple times by using the
fact that the output strbuf was only ever simply appended to and
copying the added string from the previous run.  That prerequisite
is no longer given; we now have modfiers like %< and %+ that can
cause the cache to lose track of the correct offsets.  Remove it.

Reported-by: Michael Giuffrida <michaelpg@chromium.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 11:40:53 -07:00
Junio C Hamano
c809496c97 Merge branch 'jk/interpret-branch-name'
"git branch @" created refs/heads/@ as a branch, and in general the
code that handled @{-1} and @{upstream} was a bit too loose in
disambiguating.

* jk/interpret-branch-name:
  checkout: restrict @-expansions when finding branch
  strbuf_check_ref_format(): expand only local branches
  branch: restrict @-expansions when deleting
  t3204: test git-branch @-expansion corner cases
  interpret_branch_name: allow callers to restrict expansions
  strbuf_branchname: add docstring
  strbuf_branchname: drop return value
  interpret_branch_name: move docstring to header file
  interpret_branch_name(): handle auto-namelen for @{-1}
2017-03-14 15:23:18 -07:00
Junio C Hamano
fc32293502 Merge branch 'rs/strbuf-add-real-path'
An helper function to make it easier to append the result from
real_path() to a strbuf has been added.

* rs/strbuf-add-real-path:
  strbuf: add strbuf_add_real_path()
  cocci: use ALLOC_ARRAY
2017-03-10 13:24:23 -08:00
Jeff King
0e9f62dab9 interpret_branch_name: allow callers to restrict expansions
The interpret_branch_name() function converts names like
@{-1} and @{upstream} into branch names. The expanded ref
names are not fully qualified, and may be outside of the
refs/heads/ namespace (e.g., "@" expands to "HEAD", and
"@{upstream}" is likely to be in "refs/remotes/").

This is OK for callers like dwim_ref() which are primarily
interested in resolving the resulting name, no matter where
it is. But callers like "git branch" treat the result as a
branch name in refs/heads/.  When we expand to a ref outside
that namespace, the results are very confusing (e.g., "git
branch @" tries to create refs/heads/HEAD, which is
nonsense).

Callers can't know from the returned string how the
expansion happened (e.g., did the user really ask for a
branch named "HEAD", or did we do a bogus expansion?). One
fix would be to return some out-parameters describing the
types of expansion that occurred. This has the benefit that
the caller can generate precise error messages ("I
understood @{upstream} to mean origin/master, but that is a
remote tracking branch, so you cannot create it as a local
name").

However, out-parameters make the function interface somewhat
cumbersome. Instead, let's do the opposite: let the caller
tell us which elements to expand. That's easier to pass in,
and none of the callers give more precise error messages
than "@{upstream} isn't a valid branch name" anyway (which
should be sufficient).

The strbuf_branchname() function needs a similar parameter,
as most of the callers access interpret_branch_name()
through it.

We can break the callers down into two groups:

  1. Callers that are happy with any kind of ref in the
     result. We pass "0" here, so they continue to work
     without restrictions. This includes merge_name(),
     the reflog handling in add_pending_object_with_path(),
     and substitute_branch_name(). This last is what powers
     dwim_ref().

  2. Callers that have funny corner cases (mostly in
     git-branch and git-checkout). These need to make use of
     the new parameter, but I've left them as "0" in this
     patch, and will address them individually in follow-on
     patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
Jeff King
0705fe202d strbuf_branchname: add docstring
This function and its companion, strbuf_check_branch_ref(),
did not have their purpose or semantics explained. Let's do
so.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
Jeff King
311fc74826 strbuf_branchname: drop return value
The return value from strbuf_branchname() is confusing and
useless: it's 0 if the whole name was consumed by an @-mark,
but otherwise is the length of the original name we fed.

No callers actually look at the return value, so let's just
get rid of it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
René Scharfe
33ad9ddd0b strbuf: add strbuf_add_real_path()
Add a function for appending the canonized absolute pathname of a given
path to a strbuf.  It keeps the existing contents intact, as expected of
a function of the strbuf_add() family, while avoiding copying the result
if the given strbuf is empty.  It's more consistent with the rest of the
strbuf API than strbuf_realpath(), which it's wrapping.

Also add a semantic patch demonstrating its intended usage and apply it
to the current tree.  Using strbuf_add_real_path() instead of calling
strbuf_addstr() and real_path() avoids an extra copy to a static buffer.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-27 11:02:06 -08:00
René Scharfe
35d803bc9a use SWAP macro
Apply the semantic patch swap.cocci to convert hand-rolled swaps to use
the macro SWAP.  The resulting code is shorter and easier to read, the
object code is effectively unchanged.

The patch for object.c had to be hand-edited in order to preserve the
comment before the change; Coccinelle tried to eat it for some reason.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-30 14:17:00 -08:00
Jeff King
670c359da3 link_alt_odb_entry: handle normalize_path errors
When we add a new alternate to the list, we try to normalize
out any redundant "..", etc. However, we do not look at the
return value of normalize_path_copy(), and will happily
continue with a path that could not be normalized. Worse,
the normalizing process is done in-place, so we are left
with whatever half-finished working state the normalizing
function was in.

Fortunately, this cannot cause us to read past the end of
our buffer, as that working state will always leave the
NUL from the original path in place. And we do tend to
notice problems when we check is_directory() on the path.
But you can see the nonsense that we feed to is_directory
with an entry like:

  this/../../is/../../way/../../too/../../deep/../../to/../../resolve

in your objects/info/alternates, which yields:

  error: object directory
  /to/e/deep/too/way//ects/this/../../is/../../way/../../too/../../deep/../../to/../../resolve
  does not exist; check .git/objects/info/alternates.

We can easily fix this just by checking the return value.
But that makes it hard to generate a good error message,
since we're normalizing in-place and our input value has
been overwritten by cruft.

Instead, let's provide a strbuf helper that does an in-place
normalize, but restores the original contents on error. This
uses a second buffer under the hood, which is slightly less
efficient, but this is not a performance-critical code path.

The strbuf helper can also properly set the "len" parameter
of the strbuf before returning. Just doing:

  normalize_path_copy(buf.buf, buf.buf);

will shorten the string, but leave buf.len at the original
length. That may be confusing to later code which uses the
strbuf.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10 13:52:36 -07:00
Junio C Hamano
48aa37ed42 Merge branch 'rs/use-strbuf-addbuf' into maint
Code cleanup.

* rs/use-strbuf-addbuf:
  strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
  use strbuf_addbuf() for appending a strbuf to another
2016-08-08 14:21:42 -07:00
Junio C Hamano
b4e8a847ba Merge branch 'rs/use-strbuf-addbuf'
Code cleanup.

* rs/use-strbuf-addbuf:
  strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
  use strbuf_addbuf() for appending a strbuf to another
2016-07-25 14:13:47 -07:00
René Scharfe
31471ba21e strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
Implement strbuf_addbuf() as a normal function in order to avoid calling
strbuf_grow() twice, with the second callinside strbud_add() being a
no-op.  This is slightly faster and also reduces the text size a bit.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-22 09:22:26 -07:00
Junio C Hamano
f7927316cf Merge branch 'pb/strbuf-read-file-doc' into maint
Minor doc update.

* pb/strbuf-read-file-doc:
  strbuf: describe the return value of strbuf_read_file
2016-07-06 13:06:45 -07:00
Junio C Hamano
ed319fca33 Merge branch 'pb/strbuf-read-file-doc'
* pb/strbuf-read-file-doc:
  strbuf: describe the return value of strbuf_read_file
2016-06-27 09:56:46 -07:00
Pranit Bauva
ed008d7bb9 strbuf: describe the return value of strbuf_read_file
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-14 10:57:21 -07:00
Junio C Hamano
bdebbeb334 Merge branch 'sb/submodule-parallel-update'
A major part of "git submodule update" has been ported to C to take
advantage of the recently added framework to run download tasks in
parallel.

* sb/submodule-parallel-update:
  clone: allow an explicit argument for parallel submodule clones
  submodule update: expose parallelism to the user
  submodule helper: remove double 'fatal: ' prefix
  git submodule update: have a dedicated helper for cloning
  run_processes_parallel: rename parameters for the callbacks
  run_processes_parallel: treat output of children as byte array
  submodule update: direct error message to stderr
  fetching submodules: respect `submodule.fetchJobs` config option
  submodule-config: drop check against NULL
  submodule-config: keep update strategy around
2016-04-06 11:39:01 -07:00
Stefan Beller
2dac9b5637 run_processes_parallel: treat output of children as byte array
We do not want the output to be interrupted by a NUL byte, so we
cannot use raw fputs. Introduce strbuf_write to avoid having long
arguments in run-command.c.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-01 11:57:19 -08:00
Junio C Hamano
b62624b51a Merge branch 'jc/strbuf-getline'
The preliminary clean-up for jc/peace-with-crlf topic.

* jc/strbuf-getline:
  strbuf: give strbuf_getline() to the "most text friendly" variant
  checkout-index: there are only two possible line terminations
  update-index: there are only two possible line terminations
  check-ignore: there are only two possible line terminations
  check-attr: there are only two possible line terminations
  mktree: there are only two possible line terminations
  strbuf: introduce strbuf_getline_{lf,nul}()
  strbuf: make strbuf_getline_crlf() global
  strbuf: miniscule style fix
2016-01-28 16:10:14 -08:00
Junio C Hamano
1a0c8dfd89 strbuf: give strbuf_getline() to the "most text friendly" variant
Now there is no direct caller to strbuf_getline(), we can demote it
to file-scope static that is private to strbuf.c and rename it to
strbuf_getdelim().  Rename strbuf_getline_crlf(), which is designed
to be the most "text friendly" variant, and allow it to take over
this simplest name, strbuf_getline(), so we can add more uses of it
without having to type _crlf over and over again in the coming
steps.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15 10:23:57 -08:00
Junio C Hamano
8f309aeb82 strbuf: introduce strbuf_getline_{lf,nul}()
The strbuf_getline() interface allows a byte other than LF or NUL as
the line terminator, but this is only because I wrote these
codepaths anticipating that there might be a value other than NUL
and LF that could be useful when I introduced line_termination long
time ago.  No useful caller that uses other value has emerged.

By now, it is clear that the interface is overly broad without a
good reason.  Many codepaths have hardcoded preference to read
either LF terminated or NUL terminated records from their input, and
then call strbuf_getline() with LF or NUL as the third parameter.

This step introduces two thin wrappers around strbuf_getline(),
namely, strbuf_getline_lf() and strbuf_getline_nul(), and
mechanically rewrites these call sites to call either one of
them.  The changes contained in this patch are:

 * introduction of these two functions in strbuf.[ch]

 * mechanical conversion of all callers to strbuf_getline() with
   either '\n' or '\0' as the third parameter to instead call the
   respective thin wrapper.

After this step, output from "git grep 'strbuf_getline('" would
become a lot smaller.  An interim goal of this series is to make
this an empty set, so that we can have strbuf_getline_crlf() take
over the shorter name strbuf_getline().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15 10:12:51 -08:00
Junio C Hamano
c8aa9fdf5d strbuf: make strbuf_getline_crlf() global
Often we read "text" files that are supplied by the end user
(e.g. commit log message that was edited with $GIT_EDITOR upon 'git
commit -e'), and in some environments lines in a text file are
terminated with CRLF.  Existing strbuf_getline() knows to read a
single line and then strip the terminating byte from the result, but
it is handy to have a version that is more tailored for a "text"
input that takes both '\n' and '\r\n' as line terminator (aka
<newline> in POSIX lingo) and returns the body of the line after
stripping <newline>.

Recently reimplemented "git am" uses such a function implemented
privately; move it to strbuf.[ch] and make it available for others.

Note that we do not blindly replace calls to strbuf_getline() that
uses LF as the line terminator with calls to strbuf_getline_crlf()
and this is very much deliberate.  Some callers may want to treat an
incoming line that ends with CR (and terminated with LF) to have a
payload that includes the final CR, and such a blind replacement
will result in misconversion when done without code audit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-14 15:05:55 -08:00
Stefan Beller
b4e04fb66e strbuf: add strbuf_read_once to read without blocking
The new call will read from a file descriptor into a strbuf once. The
underlying call xread is just run once. xread only reattempts
reading in case of EINTR, which makes it suitable to use for a
nonblocking read.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-16 12:06:08 -08:00
Junio C Hamano
1ad7c0f689 Merge branch 'tk/stripspace'
The internal stripspace() function has been moved to where it
logically belongs to, i.e. strbuf API, and the command line parser
of "git stripspace" has been updated to use the parse_options API.

* tk/stripspace:
  stripspace: use parse-options for command-line parsing
  strbuf: make stripspace() part of strbuf
2015-10-26 15:55:20 -07:00
Tobias Klauser
63af4a8446 strbuf: make stripspace() part of strbuf
This function is also used in other builtins than stripspace, so it
makes sense to have it in a more generic place.  Since it operates
on an strbuf and the function is declared in strbuf.h, move it to
strbuf.c and add the corresponding prefix to its name, just like
other API functions in the strbuf_* family.

Also switch all current users of stripspace() to the new function
name and keep a temporary wrapper inline function for any topic
branches still using stripspace().

Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-16 09:45:15 -07:00
Jeff King
af49c6d091 add reentrant variants of sha1_to_hex and find_unique_abbrev
The sha1_to_hex and find_unique_abbrev functions always
write into reusable static buffers. There are a few problems
with this:

  - future calls overwrite our result. This is especially
    annoying with find_unique_abbrev, which does not have a
    ring of buffers, so you cannot even printf() a result
    that has two abbreviated sha1s.

  - if you want to put the result into another buffer, we
    often strcpy, which looks suspicious when auditing for
    overflows.

This patch introduces sha1_to_hex_r and find_unique_abbrev_r,
which write into a user-provided buffer. Of course this is
just punting on the overflow-auditing, as the buffer
obviously needs to be GIT_SHA1_HEXSZ + 1 bytes. But it is
much easier to audit, since that is a well-known size.

We retain the non-reentrant forms, which just become thin
wrappers around the reentrant ones. This patch also adds a
strbuf variant of find_unique_abbrev, which will be handy in
later patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 10:18:18 -07:00
Jeff King
399ad553ce strbuf: make strbuf_complete_line more generic
The strbuf_complete_line function makes sure that a buffer
ends in a newline. But we may want to do this for any
character (e.g., "/" on the end of a path). Let's factor out
a generic version, and keep strbuf_complete_line as a thin
wrapper.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 10:18:18 -07:00
Junio C Hamano
d939af12bd Merge branch 'jk/date-mode-format'
Teach "git log" and friends a new "--date=format:..." option to
format timestamps using system's strftime(3).

* jk/date-mode-format:
  strbuf: make strbuf_addftime more robust
  introduce "format" date-mode
  convert "enum date_mode" into a struct
  show-branch: use DATE_RELATIVE instead of magic number
2015-08-03 11:01:27 -07:00
Junio C Hamano
d790ba92cc Merge branch 'mh/strbuf-read-file-returns-ssize-t'
Avoid possible ssize_t to int truncation.

* mh/strbuf-read-file-returns-ssize-t:
  strbuf: strbuf_read_file() should return ssize_t
2015-07-13 14:00:27 -07:00
Michael Haggerty
6c8afe495b strbuf: strbuf_read_file() should return ssize_t
It is currently declared to return int, which could overflow for
large files.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-03 18:25:02 -07:00
Jeff King
aa1462cc3d introduce "format" date-mode
This feeds the format directly to strftime. Besides being a
little more flexible, the main advantage is that your system
strftime may know more about your locale's preferred format
(e.g., how to spell the days of the week).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-29 11:39:10 -07:00
Jeff King
fec501dae8 strbuf_addch: avoid calling strbuf_grow
We mark strbuf_addch as inline, because we expect it may be
called from a tight loop. However, the first thing it does
is call the non-inline strbuf_grow(), which can handle
arbitrary-sized growth. Since we know that we only need a
single character, we can use the inline strbuf_avail() to
quickly check whether we need to grow at all.

Our check is redundant when we do call strbuf_grow(), but
that's OK. The common case is that we avoid calling it at
all, and we have made that case faster.

On a silly pathological case:

  perl -le '
    print "[core]";
    print "key$_ = value$_" for (1..1000000)
  ' >input
  git config -f input core.key1

this dropped the time to run git-config from:

  real    0m0.159s
  user    0m0.152s
  sys     0m0.004s

to:

  real    0m0.140s
  user    0m0.136s
  sys     0m0.004s

for a savings of 12%.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-04-16 08:15:05 -07:00
Jeff King
d468fa2721 strbuf.h: group documentation for trim functions
The relationship between these makes more sense if you read
them as a group, which can help people who are looking for
the right function. Let's give them a single comment.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:47 -08:00
Jeff King
f20e56e202 strbuf.h: drop boilerplate descriptions of strbuf_split_*
The description of strbuf_split_buf says most of what
needs to be said for all of the split variants that take
strings, raw memory, etc. We have a boilerplate comment
above each that points to the first. This boilerplate
ends up making it harder to read, because it spaces out the
functions, which could otherwise be read as a group.

Let's drop the boilerplate completely, and mention the
variants in the top comment. This is perhaps slightly worse
for a hypothetical system which pulls the documentation for
each function out of the comment immediately preceding it.
But such a system does not yet exist, and anyway, the end
result of extracting the boilerplate comments would not lead
to a very easy-to-read result.  We would do better in the
long run to teach the extraction system about groups of
related functions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:47 -08:00
Jeff King
14e2177a40 strbuf.h: reorganize api function grouping headers
The original API doc had something like:

    Functions
    ---------

    * Life cycle

      ... some life-cycle functions ...

    * Related to the contents of the buffer

      ... functions related to contents ....

    etc

This grouping can be hard to read in the comment sources,
given the "*" in the comment lines, and the amount of text
between each section.

Instead, let's make a flat list of groupings, and underline
each as a section header. That makes them stand out, and
eliminates the weird half-phrase of "Related to...". Like:

    Functions related to the contents of the buffer
    -----------------------------------------------

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:47 -08:00
Jeff King
088c9a86ff strbuf.h: format asciidoc code blocks as 4-space indent
This is much easier to read when the whole thing is stuffed
inside a comment block. And there is precedent for this
convention in markdown (and just in general ascii text).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:47 -08:00
Jeff King
aa07cac43f strbuf.h: drop asciidoc list formatting from API docs
Using a hanging indent is much more readable. This means we
won't format as asciidoc anymore, but since we don't have a
working system for extracting these comments anyway, it's
probably more important to just make the source readable.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:46 -08:00
Stefan Beller
6afbbdda33 strbuf.h: unify documentation comments beginnings
The prior patch uses "/**" to denote "documentation"
comments that we pulled from api-strbuf.txt. Let's use a
consistent style for similar comments that were already in
strbuf.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:46 -08:00
Jeff King
bdfdaa4978 strbuf.h: integrate api-strbuf.txt documentation
Some of strbuf is documented as comments above functions,
and some is separate in Documentation/technical/api-strbuf.txt.
This makes it annoying to find the appropriate documentation.
We'd rather have it all in one place, which means all in the
text document, or all in the header.

Let's choose the header as that place. Even though the
formatting is not quite as pretty, this keeps the
documentation close to the related code.  The hope is that
this makes it easier to find what you want (human-readable
comments are right next to the C declarations), and easier
for writers to keep the documentation up to date.

This is more or less a straight import of the text from
api-strbuf.txt into C comments, complete with asciidoc
formatting. The exceptions are:

 1. All comments created in this way are started with "/**"
    to indicate they are part of the API documentation. This
    may help later with extracting the text to pretty-print
    it.

 2. Function descriptions do not repeat the function name,
    as it is available in the context directly below.  So:

      `strbuf_add`::

          Add data of given length to the buffer.

    from api-strbuf.txt becomes:

      /**
       * Add data of given length to the buffer.
       */
      void strbuf_add(struct strbuf *sb, const void *, size_t);

    As a result, any block-continuation required in asciidoc
    for that list item was dropped in favor of straight
    blank-line paragraph (since it is not necessary when we
    are not in a list item).

 3. There is minor re-wording to integrate existing comments
    and api-strbuf text. In each case, I took whichever
    version was more descriptive, and eliminated any
    redundancies. In one case, for strbuf_addstr, the api
    documentation gave its inline definition; I eliminated
    this as redundant with the actual definition, which can
    be seen directly below the comment.

 4. The functions in the header file are re-ordered to match
    the ordering of the API documentation, under the
    assumption that more thought went into the grouping
    there.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-16 14:40:46 -08:00
Junio C Hamano
56feed1c76 Merge branch 'rs/export-strbuf-addchars'
Code clean-up.

* rs/export-strbuf-addchars:
  strbuf: use strbuf_addchars() for adding a char multiple times
  strbuf: export strbuf_addchars()
2014-09-19 11:38:39 -07:00
René Scharfe
d07235a027 strbuf: export strbuf_addchars()
Move strbuf_addchars() to strbuf.c, where it belongs, and make it
available for other callers.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-08 11:26:45 -07:00
Junio C Hamano
f655651e09 Merge branch 'rs/strbuf-getcwd'
Reduce the use of fixed sized buffer passed to getcwd() calls
by introducing xgetcwd() helper.

* rs/strbuf-getcwd:
  use strbuf_add_absolute_path() to add absolute paths
  abspath: convert absolute_path() to strbuf
  use xgetcwd() to set $GIT_DIR
  use xgetcwd() to get the current directory or die
  wrapper: add xgetcwd()
  abspath: convert real_path_internal() to strbuf
  abspath: use strbuf_getcwd() to remember original working directory
  setup: convert setup_git_directory_gently_1 et al. to strbuf
  unix-sockets: use strbuf_getcwd()
  strbuf: add strbuf_getcwd()
2014-09-02 13:28:44 -07:00
René Scharfe
679eebe24d abspath: convert absolute_path() to strbuf
Move most of the code of absolute_path() into the new function
strbuf_add_absolute_path() and in the process transform it to use
struct strbuf and xgetcwd() instead of a PATH_MAX-sized buffer,
which can be too small on some file systems.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-26 11:06:06 -07:00