Commit Graph

137 Commits

Author SHA1 Message Date
Junio C Hamano
a741e2825b Merge branch 'jd/fix-strbuf-add-urlencode-bytes'
Bytes with high-bit set were encoded incorrectly and made
credential helper fail.

* jd/fix-strbuf-add-urlencode-bytes:
  strbuf: fix urlencode format string on signed char
2018-01-05 13:28:10 -08:00
Junio C Hamano
f427b94985 Merge branch 'cc/skip-to-optional-val'
Introduce a helper to simplify code to parse a common pattern that
expects either "--key" or "--key=<something>".

* cc/skip-to-optional-val:
  t4045: reindent to make helpers readable
  diff: add tests for --relative without optional prefix value
  diff: use skip_to_optional_arg_default() in parsing --relative
  diff: use skip_to_optional_arg_default()
  diff: use skip_to_optional_arg()
  index-pack: use skip_to_optional_arg()
  git-compat-util: introduce skip_to_optional_arg()
2017-12-28 14:08:46 -08:00
Junio C Hamano
a13e45f1e7 Merge branch 'rs/strbuf-read-once-reset-length'
Leakfix.

* rs/strbuf-read-once-reset-length:
  strbuf: release memory on read error in strbuf_read_once()
2017-12-27 11:16:24 -08:00
Julien Dusser
4c267f2ae3 strbuf: fix urlencode format string on signed char
Git credential fails with special char in password with

    remote: Invalid username or password.
    fatal: Authentication failed for

    File ~/.git-credential contains badly urlencoded characters
    %ffffffXX%ffffffYY instead of %XX%YY.

Add a cast to an unsigned char to fix urlencode use of %02x on a
char.

Signed-off-by: Julien Dusser <julien.dusser@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22 13:43:19 -08:00
Christian Couder
afaef55e23 git-compat-util: introduce skip_to_optional_arg()
We often accept both a "--key" option and a "--key=<val>" option.

These options currently are parsed using something like:

if (!strcmp(arg, "--key")) {
	/* do something */
} else if (skip_prefix(arg, "--key=", &arg)) {
	/* do something with arg */
}

which is a bit cumbersome compared to just:

if (skip_to_optional_arg(arg, "--key", &arg)) {
	/* do something with arg */
}

This also introduces skip_to_optional_arg_default() for the few
cases where something different should be done when the first
argument is exactly "--key" than when it is exactly "--key=".

In general it is better for UI consistency and simplicity if
"--key" and "--key=" do the same thing though, so that using
skip_to_optional_arg() should be encouraged compared to
skip_to_optional_arg_default().

Note that these functions can be used to parse any "key=value"
string where "key" is also considered as valid, not just
command line options.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-11 16:10:12 -08:00
René Scharfe
c3ff8f6c14 strbuf: release memory on read error in strbuf_read_once()
If other strbuf add functions cause the first allocation and
subsequently encounter an error then they release the memory, restoring
the pristine state of the strbuf.  That simplifies error handling for
callers.

Do the same in strbuf_read_once(), and do it also in case no bytes were
read -- which may or may not be an error as well, depending on the
caller.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-07 13:19:23 -08:00
Junio C Hamano
0c493966ff Merge branch 'rs/strbuf-getwholeline-fix'
A helper function to read a single whole line into strbuf
mistakenly triggered OOM error at EOF under certain conditions,
which has been fixed.

* rs/strbuf-getwholeline-fix:
  strbuf: clear errno before calling getdelim(3)
2017-08-22 10:29:15 -07:00
René Scharfe
642956cf45 strbuf: clear errno before calling getdelim(3)
getdelim(3) returns -1 at the end of the file and if it encounters an
error, but sets errno only in the latter case.  Set errno to zero before
calling it to avoid misdiagnosing an out-of-memory condition due to a
left-over value from some other function call.

Reported-by: Yaroslav Halchenko <yoh@onerussian.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-10 14:41:51 -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
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
49a8fe8e96 Merge branch 'rs/freebsd-getcwd-workaround'
FreeBSD implementation of getcwd(3) behaved differently when an
intermediate directory is unreadable/unsearchable depending on the
length of the buffer provided, which our strbuf_getcwd() was not
aware of.  strbuf_getcwd() has been taught to cope with it better.

* rs/freebsd-getcwd-workaround:
  strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD
2017-03-30 14:07:15 -07:00
René Scharfe
a54e938e5b strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD
FreeBSD implements getcwd(3) as a syscall, but falls back to a version
based on readdir(3) if it fails for some reason.  The latter requires
permissions to read and execute path components, while the former does
not.  That means that if our buffer is too small and we're missing
rights we could get EACCES, but we may succeed with a bigger buffer.

Keep retrying if getcwd(3) indicates lack of permissions until our
buffer can fit PATH_MAX bytes, as that's the maximum supported by the
syscall on FreeBSD anyway.  This way we do what we can to be able to
benefit from the syscall, but we also won't loop forever if there is a
real permission issue.

This fixes a regression introduced with 7333ed17 (setup: convert
setup_git_directory_gently_1 et al. to strbuf, 2014-07-28) for paths
longer than 127 bytes with components that miss read or execute
permissions (e.g. 0711 on /home for privacy reasons); we used a fixed
PATH_MAX-sized buffer before.

Reported-by: Zenobiusz Kunegunda <zenobiusz.kunegunda@interia.pl>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-26 17:41:05 -07: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
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
b0af481993 Merge branch 'rs/strbuf-remove-fix' into maint
Code cleanup.

* rs/strbuf-remove-fix:
  strbuf: use valid pointer in strbuf_remove()
2016-09-29 16:49:35 -07:00
Junio C Hamano
3ba0bbb901 Merge branch 'rs/strbuf-remove-fix'
Code cleanup.

* rs/strbuf-remove-fix:
  strbuf: use valid pointer in strbuf_remove()
2016-09-21 15:15:25 -07:00
René Scharfe
a8342a417e strbuf: use valid pointer in strbuf_remove()
The fourth argument of strbuf_splice() is passed to memcpy(3), which is
not supposed to handle NULL pointers.  Let's be extra careful and use a
valid empty string instead.  It even shortens the source code. :)

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 16:07:37 -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
f55f97cb33 Merge branch 'jk/getwholeline-getdelim-empty' into maint
strbuf_getwholeline() did not NUL-terminate the buffer on certain
corner cases in its error codepath.

* jk/getwholeline-getdelim-empty:
  strbuf_getwholeline: NUL-terminate getdelim buffer on error
2016-04-14 18:57:46 -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
Junio C Hamano
087f171f14 Merge branch 'jk/getwholeline-getdelim-empty'
strbuf_getwholeline() did not NUL-terminate the buffer on certain
corner cases in its error codepath.

* jk/getwholeline-getdelim-empty:
  strbuf_getwholeline: NUL-terminate getdelim buffer on error
2016-04-03 10:29:34 -07:00
Jeff King
b70904306f strbuf_getwholeline: NUL-terminate getdelim buffer on error
Commit 0cc30e0 (strbuf_getwholeline: use getdelim if it is
available, 2015-04-16) tries to clean up after getdelim()
returns EOF, but gets one case wrong, which can lead in some
obscure cases to us reading uninitialized memory.

After getdelim() returns -1, we re-initialize the strbuf
only if sb->buf is NULL. The thinking was that either:

  1. We fed an existing allocated buffer to getdelim(), and
     at most it would have realloc'd, leaving our NUL in
     place.

  2. We didn't have a buffer to feed, so we gave getdelim()
     NULL; sb->buf will remain NULL, and we just want to
     restore the empty slopbuf.

But that second case isn't quite right. getdelim() may
allocate a buffer, write nothing into it, and then return
EOF. The resulting strbuf rightfully has sb->len set to "0",
but is missing the NUL terminator in the first byte.

Most call-sites are fine with this. They see the EOF and
don't bother looking at the strbuf. Or they notice that
sb->len is empty, and don't look at the contents. But
there's at least one case that does neither, and relies on
parsing the resulting (possibly zero-length) string:
fast-import. You can see this in action with the new test
(though we probably only notice failure there when run with
--valgrind or ASAN).

We can fix this by unconditionally resetting the strbuf when
we have a buffer after getdelim(). That fixes case 2 above.
Case 1 is probably already fine in practice, but it does not
hurt for us to re-assert our invariants (especially because
we are relying on whatever getdelim() happens to do, which
may vary from platform to platform). Our fix covers that
case, too.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-05 10:57:37 -08: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
11529ecec9 Merge branch 'jk/tighten-alloc'
Update various codepaths to avoid manually-counted malloc().

* jk/tighten-alloc: (22 commits)
  ewah: convert to REALLOC_ARRAY, etc
  convert ewah/bitmap code to use xmalloc
  diff_populate_gitlink: use a strbuf
  transport_anonymize_url: use xstrfmt
  git-compat-util: drop mempcpy compat code
  sequencer: simplify memory allocation of get_message
  test-path-utils: fix normalize_path_copy output buffer size
  fetch-pack: simplify add_sought_entry
  fast-import: simplify allocation in start_packfile
  write_untracked_extension: use FLEX_ALLOC helper
  prepare_{git,shell}_cmd: use argv_array
  use st_add and st_mult for allocation size computation
  convert trivial cases to FLEX_ARRAY macros
  use xmallocz to avoid size arithmetic
  convert trivial cases to ALLOC_ARRAY
  convert manual allocations to argv_array
  argv-array: add detach function
  add helpers for allocating flex-array structs
  harden REALLOC_ARRAY and xcalloc against size_t overflow
  tree-diff: catch integer overflow in combine_diff_path allocation
  ...
2016-02-26 13:37:16 -08:00
Jeff King
3733e69464 use xmallocz to avoid size arithmetic
We frequently allocate strings as xmalloc(len + 1), where
the extra 1 is for the NUL terminator. This can be done more
simply with xmallocz, which also checks for integer
overflow.

There's no case where switching xmalloc(n+1) to xmallocz(n)
is wrong; the result is the same length, and malloc made no
guarantees about what was in the buffer anyway. But in some
cases, we can stop manually placing NUL at the end of the
allocated buffer. But that's only safe if it's clear that
the contents will always fill the buffer.

In each case where this patch does so, I manually examined
the control flow, and I tried to err on the side of caution.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-22 14:51:09 -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
Junio C Hamano
dce80bd18c strbuf: miniscule style fix
We write one SP on each side of an operator, even inside an [] pair
that computes the array index.

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
5096d4909f convert trivial sprintf / strcpy calls to xsnprintf
We sometimes sprintf into fixed-size buffers when we know
that the buffer is large enough to fit the input (either
because it's a constant, or because it's numeric input that
is bounded in size). Likewise with strcpy of constant
strings.

However, these sites make it hard to audit sprintf and
strcpy calls for buffer overflows, as a reader has to
cross-reference the size of the array with the input. Let's
use xsnprintf instead, which communicates to a reader that
we don't expect this to overflow (and catches the mistake in
case we do).

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
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
Junio C Hamano
3b281d1281 Merge branch 'jh/strbuf-read-use-read-in-full'
strbuf_read() used to have one extra iteration (and an unnecessary
strbuf_grow() of 8kB), which was eliminated.

* jh/strbuf-read-use-read-in-full:
  strbuf_read(): skip unnecessary strbuf_grow() at eof
2015-08-25 14:57:06 -07:00
Jim Hill
3ebbd00cf3 strbuf_read(): skip unnecessary strbuf_grow() at eof
The loop in strbuf_read() uses xread() repeatedly while extending
the strbuf until the call returns zero.  If the buffer is
sufficiently large to begin with, this results in xread()
returning the remainder of the file to the end (returning
non-zero), the loop extending the strbuf, and then making another
call to xread() to have it return zero.

By using read_in_full(), we can tell when the read reached the end
of file: when it returns less than was requested, it's eof.  This
way we can avoid an extra iteration that allocates an extra 8kB
that is never used.

Signed-off-by: Jim Hill <gjthill@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-10 12:51:13 -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
Jeff King
e4f031e34b strbuf: make strbuf_addftime more robust
The return value of strftime is poorly designed; when it
returns 0, the caller cannot tell if the buffer was not
large enough, or if the output was actually 0 bytes. In the
original implementation of strbuf_addftime, we simply punted
and guessed that our 128-byte hint would be large enough.

We can do better, though, if we're willing to treat strftime
like less of a black box. We can munge the incoming format
to make sure that it never produces 0-length output, and
then "fix" the resulting output.  That lets us reliably grow
the buffer based on strftime's return value.

Clever-idea-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-20 22:17:06 -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
0cc30e0e84 strbuf_getwholeline: use getdelim if it is available
We spend a lot of time in strbuf_getwholeline in a tight
loop reading characters from a stdio handle into a buffer.
The libc getdelim() function can do this for us with less
overhead. It's in POSIX.1-2008, and was a GNU extension
before that. Therefore we can't rely on it, but can fall
back to the existing getc loop when it is not available.

The HAVE_GETDELIM knob is turned on automatically for Linux,
where we have glibc. We don't need to set any new
feature-test macros, because we already define _GNU_SOURCE.
Other systems that implement getdelim may need to other
macros (probably _POSIX_C_SOURCE >= 200809L), but we can
address that along with setting the Makefile knob after
testing the feature on those systems.

Running "git rev-parse refs/heads/does-not-exist" on a repo
with an extremely large (1.6GB) packed-refs file went from
(best-of-5):

  real    0m8.601s
  user    0m8.084s
  sys     0m0.524s

to:

  real    0m6.768s
  user    0m6.340s
  sys     0m0.432s

for a wall-clock speedup of 21%.

Based on a patch from Rasmus Villemoes <rv@rasmusvillemoes.dk>.

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
f80c153bea strbuf_getwholeline: avoid calling strbuf_grow
As with the recent speedup to strbuf_addch, we can avoid
calling strbuf_grow() in a tight loop of single-character
adds by instead checking strbuf_avail.

Note that we would instead call strbuf_addch directly here,
but it does more work than necessary: it will NUL-terminate
the result for each character read. Instead, in this loop we
read the characters one by one and then add the terminator
manually at the end.

Running "git rev-parse refs/heads/does-not-exist" on a repo
with an extremely large (1.6GB) packed-refs file went from
(best-of-5):

  real    0m10.948s
  user    0m10.548s
  sys     0m0.412s

to:

  real    0m8.601s
  user    0m8.084s
  sys     0m0.524s

for a wall-clock speedup of 21%.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
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
82912d1de8 strbuf_getwholeline: use getc_unlocked
strbuf_getwholeline calls getc in a tight loop. On modern
libc implementations, the stdio code locks the handle for
every operation, which means we are paying a significant
overhead.  We can get around this by locking the handle for
the whole loop and using the unlocked variant.

Running "git rev-parse refs/heads/does-not-exist" on a repo
with an extremely large (1.6GB) packed-refs file went from:

  real    0m18.900s
  user    0m18.472s
  sys     0m0.448s

to:

  real    0m10.953s
  user    0m10.384s
  sys     0m0.580s

for a wall-clock speedup of 42%. All times are best-of-3,
and done on a glibc 2.19 system.

Note that we call into strbuf_grow while holding the lock.
It's possible for that function to call other stdio
functions (e.g., printing to stderr when dying due to malloc
error); however, the POSIX.1-2001 definition of flockfile
makes it clear that the locks are per-handle, so we are fine
unless somebody else tries to read from our same handle.
This doesn't ever happen in the current code, and is
unlikely to be added in the future (we would have to do
something exotic like add a die_routine that tried to read
from stdin).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-04-16 08:15:04 -07:00