drop strcpy in favor of raw sha1_to_hex

In some cases where we strcpy() the result of sha1_to_hex(),
there's no need; the result goes directly into a printf
statement, and we can simply pass the return value from
sha1_to_hex() directly.

When this code was originally written, sha1_to_hex used a
single buffer, and it was not safe to use it twice within a
single expression. That changed as of dcb3450 (sha1_to_hex()
usage cleanup, 2006-05-03), but this code was never updated.

History-dug-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

use sha1_to_hex_r() instead of strcpy

Before sha1_to_hex_r() existed, a simple way to get hex
sha1 into a buffer was with:

  strcpy(buf, sha1_to_hex(sha1));

This isn't wrong (assuming the buf is 41 characters), but it
makes auditing the code base for bad strcpy() calls harder,
as these become false positives.

Let's convert them to sha1_to_hex_r(), and likewise for
some calls to find_unique_abbrev(). While we're here, we'll
double-check that all of the buffers are correctly sized,
and use the more obvious GIT_SHA1_HEXSZ constant.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

daemon: use cld->env_array when re-spawning

This avoids an ugly strcat into a fixed-size buffer. It's
not wrong (the buffer is plenty large enough for an IPv6
address plus some minor formatting), but it takes some
effort to verify that.

Unfortunately we are still stuck with some fixed-size
buffers to hold the output of inet_ntop. But at least we now
pass very easy-to-verify parameters, rather than doing a
manual computation to account for other data in the buffer.

As a side effect, this also fixes the case where we might
pass an uninitialized portbuf buffer through the
environment. This probably couldn't happen in practice, as
it would mean that addr->sa_family was neither AF_INET nor
AF_INET6 (and that is all we are listening on).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

stat_tracking_info: convert to argv_array

In addition to dropping the magic number for the fixed-size
argv, we can also drop a fixed-length buffer and some
strcpy's into it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-push: use an argv_array for setup_revisions

This drops the magic number for the fixed-size argv arrays,
so we do not have to wonder if we are overflowing it. We can
also drop some confusing sha1_to_hex memory allocation
(which seems to predate the ring of buffers allowing
multiple calls), and get rid of an unchecked sprintf call.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch-pack: use argv_array for index-pack / unpack-objects

This cleans up a magic number that must be kept in sync with
the rest of the code (the number of argv slots). It also
lets us drop some fixed buffers and an sprintf (since we
can now use argv_array_pushf).

We do still have to keep one fixed buffer for calling
gethostname, but at least now the size computations for it
are much simpler.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat

We dynamically allocate a buffer and then strcpy and strcat
into it. This isn't buggy, but we'd prefer to avoid these
suspicious functions.

This would be a good candidate for converstion to xstrfmt,
but we need to record the length for dealing with index
entries. A strbuf handles that for us.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

write_loose_object: convert to strbuf

When creating a loose object tempfile, we use a fixed
PATH_MAX-sized buffer, and strcpy directly into it. This
isn't buggy, because we do a rough check of the size, but
there's no verification that our guesstimate of the required
space is enough (in fact, it's several bytes too big for the
current naming scheme).

Let's switch to a strbuf, which makes this much easier to
verify. The allocation overhead should be negligible, since
we are replacing a static buffer with a static strbuf, and
we'll only need to allocate on the first call.

While we're here, we can also document a subtle interaction
with mkstemp that would be easy to overlook.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

remove_leading_path: use a strbuf for internal storage

This function strcpy's directly into a PATH_MAX-sized
buffer. There's only one caller, which feeds the git_dir into
it, so it's not easy to trigger in practice (even if you fed
a large $GIT_DIR through the environment or .git file, it
would have to actually exist and be accessible on the
filesystem to get to this point). We can fix it by moving to
a strbuf.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

enter_repo: convert fixed-size buffers to strbufs

We use two PATH_MAX-sized buffers to represent the repo
path, and must make sure not to overflow them. We do take
care to check the lengths, but the logic is rather hard to
follow, as we use several magic numbers (e.g., "PATH_MAX -
10"). And in fact you _can_ overflow the buffer if you have
a ".git" file with an extremely long path in it.

By switching to strbufs, these problems all go away. We do,
however, retain the check that the initial input we get is
no larger than PATH_MAX. This function is an entry point for
untrusted repo names from the network, and it's a good idea
to keep a sanity check (both to avoid allocating arbitrary
amounts of memory, and also as a layer of defense against
any downstream users of the names).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

merge-recursive: convert malloc / strcpy to strbuf

This would be a fairly routine use of xstrfmt, except that
we need to remember the length of the result to pass to
cache_name_pos. So just use a strbuf, which makes this
simple.

As a bonus, this gets rid of confusing references to
"pathlen+1". The "1" is for the trailing slash we added, but
that is automatically accounted for in the strbuf's len
parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

transport: use strbufs for status table "quickref" strings

We generate range strings like "1234abcd...5678efab" for use
in the the fetch and push status tables. We use fixed-size
buffers along with strcat to do so. These aren't buggy, as
our manual size computation is correct, but there's nothing
checking that this is so.  Let's switch them to strbufs
instead, which are obviously correct, and make it easier to
audit the code base for problematic calls to strcat().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

apply: convert root string to strbuf

We use manual computation and strcpy to allocate the "root"
variable. This would be much simpler using xstrfmt.  But
since we store the length, too, we can just use a strbuf,
which handles that for us.

Note that we stop distinguishing between "no root" and
"empty root" in some cases, but that's OK; the results are
the same (e.g., inserting an empty string is a noop).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

init: use strbufs to store paths

The init code predates strbufs, and uses PATH_MAX-sized
buffers along with many manual checks on intermediate sizes
(some of which make magic assumptions, such as that init
will not create a path inside .git longer than 50
characters).

We can simplify this greatly by using strbufs, which drops
some hard-to-verify strcpy calls in favor of git_path_buf.
While we're in the area, let's also convert existing calls
to git_path to the safer git_path_buf (our existing calls
were passed to pretty tame functions, and so were not a
problem, but it's easy to be consistent and safe here).

Note that we had an explicit test that "git init" rejects
long template directories. This comes from 32d1776 (init: Do
not segfault on big GIT_TEMPLATE_DIR environment variable,
2009-04-18). We can drop the test_must_fail here, as we now
accept this and need only confirm that we don't segfault,
which was the original point of the test.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

probe_utf8_pathname_composition: use internal strbuf

When we are initializing a .git directory, we may call
probe_utf8_pathname_composition to detect utf8 mangling. We
pass in a path buffer for it to use, and it blindly
strcpy()s into it, not knowing whether the buffer is large
enough to hold the result or not.

In practice this isn't a big deal, because the buffer we
pass in already contains "$GIT_DIR/config", and we append
only a few extra bytes to it. But we can easily do the right
thing just by calling git_path_buf ourselves. Technically
this results in a different pathname (before we appended our
utf8 characters to the "config" path, and now they get their
own files in $GIT_DIR), but that should not matter for our
purposes.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

precompose_utf8: drop unused variable

The result of iconv is assigned to a variable, but we never
use it (instead, we check errno and whether the function
consumed all bytes). Let's drop the assignment, as it
triggers gcc's -Wunused-but-set-variable.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1_get_pack_name: use a strbuf

We do some manual memory computation here, and there's no
check that our 60 is not overflowed by the raw sprintf (it
isn't, because the "which" parameter is never longer than
"pack"). We can simplify this greatly with a strbuf.

Technically the end result is not identical, as the original
took care not to rewrite the object directory on each call
for performance reasons.  We could do that here, too (by
saving the baselen and resetting to it), but it's not worth
the complexity; this function is not called a lot (generally
once per packfile that we open).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-walker: store url in a strbuf

We do an unchecked sprintf directly into our url buffer.
This doesn't overflow because we know that it was sized for
"$base/objects/info/http-alternates", and we are writing
"$base/objects/info/alternates", which must be smaller. But
that is not immediately obvious to a reader who is looking
for buffer overflows. Let's switch to a strbuf, so that we
do not have to think about this issue at all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-push: use strbuf instead of fwrite_buffer

The http-push code defines an fwrite_buffer function for use
as a curl callback; it just writes to a strbuf. There's no
reason we need to use it ourselves, as we know we have a
strbuf. This lets us format directly into it, rather than
dealing with an extra temporary buffer (which required
manual length computation).

While we're here, let's also remove the literal tabs from
the source in favor of "\t", which is more visually obvious.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

remote-ext: simplify git pkt-line generation

We format a pkt-line into a heap buffer, which requires
manual computation of the required size, and uses some bare
sprintf calls. We could use a strbuf instead, which would
take care of the computation for us. But it's even easier
still to use packet_write(). Besides handling the formatting
and writing for us, it fixes two things:

  1. Our manual max-size check used 0xFFFF, while technically
     LARGE_PACKET_MAX is slightly smaller than this.

  2. Our packet will now be output as part of
     GIT_TRACE_PACKET debugging.

Unfortunately packet_write() does not let us build up the
buffer progressively, so we do have to repeat ourselves a
little depending on the "vhost" setting, but the end result
is still far more readable than the original.

Since there were no tests covering this feature at all,
we'll add a few into t5802.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

upload-archive: convert sprintf to strbuf

When we report an error to the client, we format it into a
fixed-size buffer using vsprintf(). This can't actually
overflow in practice, since we only format a very tame
subset of strings (mostly strerror() output). However, it's
hard to tell immediately, so let's just use a strbuf so
readers do not have to wonder.

We do add an allocation here, but the performance is not
important; the next step is to call die() anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

resolve_ref: use strbufs for internal buffers

resolve_ref already uses a strbuf internally when generating
pathnames, but it uses fixed-size buffers for storing the
refname and symbolic refs. This means that you cannot
actually point HEAD to a ref that is larger than 256 bytes.

We can lift this limit by using strbufs here, too. Like
sb_path, we pass the the buffers into our helper function,
so that we can easily clean up all output paths. We can also
drop the "unsafe" name from our helper function, as it no
longer uses a single static buffer (but of course
resolve_ref_unsafe is still unsafe, because the static
buffers moved there).

As a bonus, we also get to drop some strcpy calls between
the two fixed buffers (that cannot currently overflow
because the two buffers are sized identically).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read_remotes_file: simplify string handling

The main motivation for this cleanup is to switch our
line-reading to a strbuf, which removes the use of a
fixed-size buffer (which limited the size of remote URLs).
Since we have the strbuf, we can make use of strbuf_rtrim().

While we're here, we can also simplify the parsing of each
line.  First, we can use skip_prefix() to avoid some magic
numbers.

But second, we can avoid splitting the parsing and actions
for each line into two stages. Right now we figure out which
type of line we have, set an int to a magic number,
skip any intermediate whitespace, and then act on
the resulting value based on the magic number.

Instead, let's factor the whitespace skipping into a
function. That lets us avoid the magic numbers and keep the
actions close to the parsing.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read_branches_file: simplify string handling

This function does a lot of manual string handling, and has
some unnecessary limits. This patch cleans up a number of
things:

  1. Drop the arbitrary 1000-byte limit on the size of the
     remote name (we do not have such a limit in any of the
     other remote-reading mechanisms).

  2. Replace fgets into a fixed-size buffer with a strbuf,
     eliminating any limits on the length of the URL.

  3. Replace manual whitespace handling with strbuf_trim
     (since we now have a strbuf). This also gets rid
     of a call to strcpy, and the confusing reuse of the "p"
     pointer for multiple purposes.

  4. We currently build up the refspecs over multiple strbuf
     calls. We do this to handle the fact that the URL "frag"
     may not be present. But rather than have multiple
     conditionals, let's just default "frag" to "master".
     This lets us format the refspecs with a single xstrfmt.
     It's shorter, and easier to see what the final string
     looks like.

     We also update the misleading comment in this area (the
     local branch is named after the remote name, not after
     the branch name on the remote side).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mailmap: replace strcpy with xstrdup

We want to make a copy of a string without any leading
whitespace. To do so, we allocate a buffer large enough to
hold the original, skip past the whitespace, then copy that.
It's much simpler to just allocate after we've skipped, in
which case we can just copy the remainder of the string,
leaving no question of whether "len" is large enough.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

help: drop prepend function in favor of xstrfmt

This function predates xstrfmt, and its functionality is a
subset. Let's just use xstrfmt.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ref-filter: drop sprintf and strcpy calls

The ref-filter code comes from for-each-ref, and inherited a
number of raw sprintf and strcpy calls. These are generally
all safe, as we custom-size the buffers, or are formatting
numbers into sufficiently large buffers. But we can make the
resulting code even simpler and more obviously correct by
using some of our helper functions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

use strip_suffix and xstrfmt to replace suffix

When we want to convert "foo.pack" to "foo.idx", we do it by
duplicating the original string and then munging the bytes
in place. Let's use strip_suffix and xstrfmt instead, which
has several advantages:

  1. It's more clear what the intent is.

  2. It does not implicitly rely on the fact that
     strlen(".idx") <= strlen(".pack") to avoid an overflow.

  3. We communicate the assumption that the input file ends
     with ".pack" (and get a run-time check that this is so).

  4. We drop calls to strcpy, which makes auditing the code
     base easier.

Likewise, we can do this to convert ".pack" to ".bitmap",
avoiding some manual memory computation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch: replace static buffer with xstrfmt

We parse the INFINITE_DEPTH constant into a static,
fixed-size buffer using sprintf. This buffer is sufficiently
large for the current constant, but it's a suspicious
pattern, as the constant is defined far away, and it's not
immediately obvious that 12 bytes are large enough to hold
it.

We can just use xstrfmt here, which gets rid of any question
of the buffer size. It also removes any concerns with object
lifetime, which means we do not have to wonder why this
buffer deep within a conditional is marked "static" (we
never free our newly allocated result, of course, but that's
OK; it's global that lasts the lifetime of the whole program
anyway).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config: use xstrfmt in normalize_value

We xmalloc a fixed-size buffer and sprintf into it; this is
OK because the size of our formatting types is finite, but
that's not immediately clear to a reader auditing sprintf
calls. Let's switch to xstrfmt, which is shorter and
obviously correct.

Note that just dropping the common xmalloc here causes gcc
to complain with -Wmaybe-uninitialized. That's because if
"types" does not match any of our known types, we never
write anything into the "normalized" pointer. With the
current code, gcc doesn't notice because we always return a
valid pointer (just one which might point to uninitialized
data, but the compiler doesn't know that). In other words,
the current code is potentially buggy if new types are added
without updating this spot.

So let's take this opportunity to clean up the function a
bit more. We can drop the "normalized" pointer entirely, and
just return directly from each code path. And then add an
assertion at the end in case we haven't covered any cases.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

replace trivial malloc + sprintf / strcpy calls with xstrfmt

It's a common pattern to do:

  foo = xmalloc(strlen(one) + strlen(two) + 1 + 1);
  sprintf(foo, "%s %s", one, two);

(or possibly some variant with strcpy()s or a more
complicated length computation).  We can switch these to use
xstrfmt, which is shorter, involves less error-prone manual
computation, and removes many sprintf and strcpy calls which
make it harder to audit the code for real buffer overflows.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

receive-pack: convert strncpy to xsnprintf

This strncpy is pointless; we pass the strlen() of the src
string, meaning that it works just like a memcpy. Worse,
though, is that the size has no relation to the destination
buffer, meaning it is a potential overflow.  In practice,
it's not. We pass only short constant strings like
"warning: " and "error: ", which are much smaller than the
destination buffer.

We can make this much simpler by just using xsnprintf, which
will check for overflow and return the size for our next
vsnprintf, without us having to run a separate strlen().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-push: replace strcat with xsnprintf

We account for these strcats in our initial allocation, but
the code is confusing to follow and verify. Let's remember
our original allocation length, and then xsnprintf can
verify that we don't exceed it.

Note that we can't just use xstrfmt here (which would be
even cleaner) because the code tries to grow the buffer only
when necessary.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

add_packed_git: convert strcpy into xsnprintf

We have the path "foo.idx", and we create a buffer big
enough to hold "foo.pack" and "foo.keep", and then strcpy
straight into it. This isn't a bug (we have enough space),
but it's very hard to tell from the strcpy that this is so.

Let's instead use strip_suffix to take off the ".idx",
record the size of our allocation, and use xsnprintf to make
sure we don't violate our assumptions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

entry.c: convert strcpy to xsnprintf

This particular conversion is non-obvious, because nobody
has passed our function the length of the destination
buffer. However, the interface to checkout_entry specifies
that the buffer must be at least TEMPORARY_FILENAME_LENGTH
bytes long, so we can check that (meaning the existing code
was not buggy, but merely worrisome to somebody reading it).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

grep: use xsnprintf to format failure message

This looks at first glance like the sprintf can overflow our
buffer, but it's actually fine; the p->origin string is
something constant and small, like "command line" or "-e
option".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

compat/hstrerror: convert sprintf to snprintf

This is a trivially correct use of sprintf, as our error
number should not be excessively long. But it's still nice
to drop an sprintf call.

Note that we cannot use xsnprintf here, because this is
compat code which does not load git-compat-util.h.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

stop_progress_msg: convert sprintf to xsnprintf

The usual arguments for using xsnprintf over sprintf apply,
but this case is a little tricky. We print to a fixed-size
buffer if we have room, and otherwise to an allocated
buffer. So there should be no overflow here, but it is still
good to communicate our intention, as well as to check our
earlier math for how much space the string will need.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

find_short_object_filename: convert sprintf to xsnprintf

We use sprintf() to format some hex data into a buffer. The
buffer is clearly long enough, and using snprintf here is
not necessary. And in fact, it does not really make anything
easier to audit, as the size we feed to snprintf accounts
for the magic extra 42 bytes found in each alt->name field
of struct alternate_object_database (which is there exactly
to do this formatting).

Still, it is nice to remove an sprintf call and replace it
with an xsnprintf and explanatory comment, which makes it
easier to audit the code base for overflows.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

use xsnprintf for generating git object headers

We generally use 32-byte buffers to format git's "type size"
header fields. These should not generally overflow unless
you can produce some truly gigantic objects (and our types
come from our internal array of constant strings). But it is
a good idea to use xsnprintf to make sure this is the case.

Note that we slightly modify the interface to
write_sha1_file_prepare, which nows uses "hdrlen" as an "in"
parameter as well as an "out" (on the way in it stores the
allocated size of the header, and on the way out it returns
the ultimate size of the header).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

archive-tar: use xsnprintf for trivial formatting

When we generate tar headers, we sprintf() values directly
into a struct with the fixed-size header values. For the
most part this is fine, as we are formatting small values
(e.g., the octal format of "mode & 0x7777" is of fixed
length). But it's still a good idea to use xsnprintf here.
It communicates to readers what our expectation is, and it
provides a run-time check that we are not overflowing the
buffers.

The one exception here is the mtime, which comes from the
epoch time of the commit we are archiving. For sane values,
this fits into the 12-byte value allocated in the header.
But since git can handle 64-bit times, if I claim to be a
visitor from the year 10,000 AD, I can overflow the buffer.
This turns out to be harmless, as we simply overflow into
the chksum field, which is then overwritten.

This case is also best as an xsnprintf. It should never come
up, short of extremely malformed dates, and in that case we
are probably better off dying than silently truncating the
date value (and we cannot expand the size of the buffer,
since it is dictated by the ustar format). Our friends in
the year 5138 (when we legitimately flip to a 12-digit
epoch) can deal with that problem then.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

compat/inet_ntop: fix off-by-one in inet_ntop4

Our compat inet_ntop4 function writes to a temporary buffer
with snprintf, and then uses strcpy to put the result into
the final "dst" buffer. We check the return value of
snprintf against the size of "dst", but fail to account for
the NUL terminator. As a result, we may overflow "dst" with
a single NUL. In practice, this doesn't happen because the
output of inet_ntop is limited, and we provide buffers that
are way oversized.

We can fix the off-by-one check easily, but while we are
here let's also use strlcpy for increased safety, just in
case there are other bugs lurking.

As a side note, this compat code seems to be BSD-derived.
Searching for "vixie inet_ntop" turns up NetBSD's latest
version of the same code, which has an identical fix (and
switches to strlcpy, too!).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-dump-cache-tree: avoid overflow of cache-tree name

When dumping a cache-tree, we sprintf sub-tree names directly
into a fixed-size buffer, which can overflow. We can
trivially fix this by converting to xsnprintf to at least
notice and die.

This probably should handle arbitrary-sized names, but
there's not much point. It's used only by the test scripts,
so the trivial fix is enough.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

progress: store throughput display in a strbuf

Coverity noticed that we strncpy() into a fixed-size buffer
without making sure that it actually ended up
NUL-terminated. This is unlikely to be a bug in practice,
since throughput strings rarely hit 32 characters, but it
would be nice to clean it up.

The most obvious way to do so is to add a NUL-terminator.
But instead, this patch switches the fixed-size buffer out
for a strbuf. At first glance this seems much less
efficient, until we realize that filling in the fixed-size
buffer is done by writing into a strbuf and copying the
result!

By writing straight to the buffer, we actually end up more
efficient:

  1. We avoid an extra copy of the bytes.

  2. Rather than malloc/free each time progress is shown, we
     can strbuf_reset and use the same buffer each time.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

trace: use strbuf for quote_crnl output

When we output GIT_TRACE_SETUP paths, we quote any
meta-characters. But our buffer to hold the result is only
PATH_MAX bytes, and we could double the size of the input
path (if every character needs quoting). We could use a
2*PATH_MAX buffer, if we assume the input will never be more
than PATH_MAX. But it's easier still to just switch to a
strbuf and not worry about whether the input can exceed
PATH_MAX or not.

The original copied the "p2" pointer to "p1", advancing
both. Since this gets rid of "p1", let's also drop "p2",
whose name is now confusing. We can just advance the
original "path" pointer.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mailsplit: make PATH_MAX buffers dynamic

There are several PATH_MAX-sized buffers in mailsplit, along
with some questionable uses of sprintf.  These are not
really of security interest, as local mailsplit pathnames
are not typically under control of an attacker, and you
could generally only overflow a few numbers at the end of a
path that approaches PATH_MAX (a longer path would choke
mailsplit long before). But it does not hurt to be careful,
and as a bonus we lift some limits for systems with
too-small PATH_MAX varibles.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fsck: use strbuf to generate alternate directories

When fsck-ing alternates, we make a copy of the alternate
directory in a fixed PATH_MAX buffer. We memcpy directly,
without any check whether we are overflowing the buffer.
This is OK if PATH_MAX is a true representation of the
maximum path on the system, because any path here will have
already been vetted by the alternates subsystem. But that is
not true on every system, so we should be more careful.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

add git_path_buf helper function

If you have a function that uses git_path a lot, but would
prefer to avoid the static buffers, it's useful to keep a
single scratch buffer locally and reuse it for each call.
You used to be able to do this with git_snpath:

  char buf[PATH_MAX];

  foo(git_snpath(buf, sizeof(buf), "foo"));
  bar(git_snpath(buf, sizeof(buf), "bar"));

but since 1a83c24, git_snpath has been replaced with
strbuf_git_path. This is good, because it removes the
arbitrary PATH_MAX limit. But using strbuf_git_path is more
awkward for two reasons:

  1. It adds to the buffer, rather than replacing it. This
     is consistent with other strbuf functions, but makes
     reuse of a single buffer more tedious.

  2. It doesn't return the buffer, so you can't format
     as part of a function's arguments.

The new git_path_buf solves both of these, so you can use it
like:

  struct strbuf buf = STRBUF_INIT;

  foo(git_path_buf(&buf, "foo"));
  bar(git_path_buf(&buf, "bar"));

  strbuf_release(&buf);

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

add xsnprintf helper function

There are a number of places in the code where we call
sprintf(), with the assumption that the output will fit into
the buffer. In many cases this is true (e.g., formatting a
number into a large buffer), but it is hard to tell
immediately from looking at the code. It would be nice if we
had some run-time check to make sure that our assumption is
correct (and to communicate to readers of the code that we
are not blindly calling sprintf, but have actually thought
about this case).

This patch introduces xsnprintf, which behaves just like
snprintf, except that it dies whenever the output is
truncated. This acts as a sort of assert() for these cases,
which can help find places where the assumption is violated
(as opposed to truncating and proceeding, which may just
silently give a wrong answer).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fsck: don't fsck alternates for connectivity-only check

Commit 02976bf (fsck: introduce `git fsck --connectivity-only`,
2015-06-22) recently gave fsck an option to perform only a
subset of the checks, by skipping the fsck_object_dir()
call. However, it does so only for the local object
directory, and we still do expensive checks on any alternate
repos. We should skip them in this case, too.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

archive-tar: fix minor indentation violation

This looks like a simple omission from 8539070 (archive-tar:
unindent write_tar_entry by one level, 2012-05-03).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mailsplit: fix FILE* leak in split_maildir

If we encounter an error while splitting a maildir, we exit
the function early, leaking the open filehandle. This isn't
a big deal, since we exit the program soon after, but it's
easy enough to be careful.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

show-branch: avoid segfault with --reflog of unborn branch

When no branch is given to the "--reflog" option, we resolve
HEAD to get the default branch. However, if HEAD points to
an unborn branch, resolve_ref returns NULL, and we later
segfault trying to access it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git 2.6-rc3

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'rj/mailmap-ramsay'

* rj/mailmap-ramsay:
  mailmap: update my entry with new email address

Merge branch 'bn/send-email-smtp-auth-error-message-fix'

Fix a minor regression brought in to "git send-email" by a recent
addition of the "--smtp-auth" option.

* bn/send-email-smtp-auth-error-message-fix:
  send-email: fix uninitialized var warning for $smtp_auth

Merge tag 'l10n-2.6.0-rnd2+de' of git://github.com/git-l10n/git-po

l10n-2.6.0-rnd2 plus de

* tag 'l10n-2.6.0-rnd2+de' of git://github.com/git-l10n/git-po: (25 commits)
  l10n: de.po: better language for one string
  l10n: de.po: translate 2 messages
  l10n: Update and review Vietnamese translation (2440t)
  l10n: fr.po v2.6.0 round 2 (2440t)
  l10n: zh_CN: for git v2.6.0 l10n round 2
  l10n: ca.po: update translation
  l10n: git.pot: v2.6.0 round 2 (3 improvements)
  l10n: de.po: translate 123 new messages
  l10n: fr.po v2.6.0 round 1 (2441t)
  l10n: sv.po: Update Swedish translation (2441t0f0u)
  l10n: zh_CN: for git v2.6.0 l10n round 1
  l10n: Updated Vietnamese translation (2441t)
  l10n: git.pot: v2.6.0 round 1 (123 new, 41 removed)
  l10n: zh_CN: Update Git Glossary: "commit message"
  l10n: zh_CN: Update Git Glossary: pickaxe
  l10n: zh_CN: Update Git Glossary: fork
  l10n: zh_CN: Update Git Glossary: tag
  l10n: zh_CN: Update Git Glossary: "dumb", "smart"
  l10n: zh_CN: Update Git Glossary: SHA-1
  l10n: zh_CN: Add Surrounding Spaces
  ...

send-email: fix uninitialized var warning for $smtp_auth

On the latest version of git-send-email, I see this error just before
running SMTP auth (I didn't provide any --smtp-auth= parameter):

  Use of uninitialized value $smtp_auth in pattern match (m//) at \
  /home/briannorris/git/git/git-send-email.perl line 1139.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

l10n: de.po: better language for one string

Just one string I think we could translate better.

Signed-off-by: Phillip Sz <phillip.szelat@gmail.com>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>

l10n: de.po: translate 2 messages

Translate 2 messages came from git.pot update in e447091
(l10n: git.pot: v2.6.0 round 2 (3 improvements)).

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Acked-by: Phillip Sz <phillip.szelat@gmail.com>

l10n: Update and review Vietnamese translation (2440t)

Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>

l10n: fr.po v2.6.0 round 2 (2440t)

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>

l10n: zh_CN: for git v2.6.0 l10n round 2

Update 2 translations (2440t0f0u) for git v2.6.0-rc2.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>

l10n: ca.po: update translation

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

l10n: git.pot: v2.6.0 round 2 (3 improvements)

Introduce three i18n improvements from the following commits:

* tag, update-ref: improve description of option "create-reflog"
* pull: don't mark values for option "rebase" for translation
* show-ref: place angle brackets around variables in usage string

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>

Merge branch 'master' of git://github.com/git-l10n/git-po

* 'master' of git://github.com/git-l10n/git-po:
  l10n: de.po: translate 123 new messages
  l10n: fr.po v2.6.0 round 1 (2441t)
  l10n: sv.po: Update Swedish translation (2441t0f0u)
  l10n: zh_CN: for git v2.6.0 l10n round 1
  l10n: Updated Vietnamese translation (2441t)
  l10n: git.pot: v2.6.0 round 1 (123 new, 41 removed)
  l10n: zh_CN: Update Git Glossary: "commit message"
  l10n: zh_CN: Update Git Glossary: pickaxe
  l10n: zh_CN: Update Git Glossary: fork
  l10n: zh_CN: Update Git Glossary: tag
  l10n: zh_CN: Update Git Glossary: "dumb", "smart"
  l10n: zh_CN: Update Git Glossary: SHA-1
  l10n: zh_CN: Add Surrounding Spaces
  l10n: zh_CN: Add translations for Git glossary
  l10n: TEAMS: stash inactive zh_CN team members
  l10n: zh_CN: Update Translation of "tag"
  l10n: zh_CN: Unify Translation of "packfile"
  l10n: zh_CN: Update Translation: "tag object"

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>

l10n: de.po: translate 123 new messages

Translate 123 new messages came from git.pot update in df0617b
(l10n: git.pot: v2.6.0 round 1 (123 new, 41 removed)).

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Acked-by: Phillip Sz <phillip.szelat@gmail.com>
Acked-by: Matthias Rüster <matthias.ruester@gmail.com>

l10n: fr.po v2.6.0 round 1 (2441t)

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>

Update RelNotes to 2.6

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Sync with 2.5.3

* maint:
  Git 2.5.3

Merge branch 'po/doc-branch-desc'

The branch descriptions that are set with "git branch --edit-description"
option were used in many places but they weren't clearly documented.

* po/doc-branch-desc:
  doc: show usage of branch description

Merge branch 'et/win32-poll-timeout'

* et/win32-poll-timeout:
  poll: honor the timeout on Win32

Merge branch 'as/config-doc-markup-fix'

* as/config-doc-markup-fix:
  Documentation/config: fix formatting for branch.*.rebase and pull.rebase

Git 2.5.3

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'dt/untracked-subdir' into maint

The experimental untracked-cache feature were buggy when paths with
a few levels of subdirectories are involved.

* dt/untracked-subdir:
  untracked cache: fix entry invalidation
  untracked-cache: fix subdirectory handling
  t7063: use --force-untracked-cache to speed up a bit
  untracked-cache: support sparse checkout

Merge branch 'br/svn-doc-include-paths-config' into maint

* br/svn-doc-include-paths-config:
  git-svn doc: mention "svn-remote.<name>.include-paths"

Merge branch 'ah/submodule-typofix-in-error' into maint

Error string fix.

* ah/submodule-typofix-in-error:
  git-submodule: remove extraneous space from error message

Merge branch 'js/maint-am-skip-performance-regression' into maint

* js/maint-am-skip-performance-regression:
  am --skip/--abort: merge HEAD/ORIG_HEAD tree into index

mailmap: update my entry with new email address

My 'demon' email address is no longer functional since, after 16+
years with demon, I have had to change my ISP. :(

Also, take the opportunity to remove my middle name, which I only
use on official documents (or in the GECOS field when creating a
user account on unix).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Update RelNotes to 2.6 to describe leftover bits since -rc2

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'js/maint-am-skip-performance-regression'

Recent versions of scripted "git am" has a performance regression in
"git am --skip" codepath, which no longer exists in the built-in
version on the 'master' front.  Fix the regression in the last
scripted version that appear in 2.5.x maintenance track and older.

* js/maint-am-skip-performance-regression:
  am --skip/--abort: merge HEAD/ORIG_HEAD tree into index

Merge branch 'ah/show-ref-usage-string'

Both "git show-ref -h" and "git show-ref --help" illustrated that the
"--exclude-existing" option makes the command read list of refs
from its standard input.  Change only the "show-ref -h" output to
have a pair of "<>" around the placeholder that designate an input
file, i.e. "git show-ref --exclude-existing < <ref-list>".

* ah/show-ref-usage-string:
  show-ref: place angle brackets around variables in usage string

Merge branch 'sg/help-group'

* sg/help-group:
  Makefile: use SHELL_PATH when running generate-cmdlist.sh

Merge branch 'rt/help-strings-fix'

* rt/help-strings-fix:
  tag, update-ref: improve description of option "create-reflog"
  pull: don't mark values for option "rebase" for translation

Git 2.6-rc2

Signed-off-by: Junio C Hamano <gitster@pobox.com>

poll: honor the timeout on Win32

Ensure that when passing a pipe, the gnulib poll replacement will not
return 0 before the timeout has passed.

Not obeying the timeout (and merely returning 0) causes pathological
behavior when preparing a packfile for a repository and taking a
long time to do so.  If poll were to return 0 immediately, this would
cause keep-alives to get sent as quickly as possible until the packfile
was created.  Such deviance from the standard would cause megabytes (or
more) of keep-alive packets to be sent.

GetTickCount is used as it is efficient, stable and monotonically
increasing.  (Neither GetSystemTime nor QueryPerformanceCounter have
all three of these properties.)

Signed-off-by: Edward Thomson <ethomson@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: show usage of branch description

The branch description will be included in 'git format-patch
--cover-letter' and in 'git pull-request' emails. It can also
be used in the automatic merge message. Tell the reader.

While here, clarify that the description may be a multi-line
explanation of the purpose of the branch's patch series.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge git://ozlabs.org/~paulus/gitk

* git://ozlabs.org/~paulus/gitk:
  gitk: Accelerators for the main menu
  gitk: Adjust the menu line numbers to compensate for the new entry
  gitk: Add a "Copy commit summary" command
  gitk: Update Bulgarian translation (307t)
  gitk: Update .po files
  gitk: Update Bulgarian translation (304t)
  gitk: Use translated version of "Command line" in getcommitlines
  gitk: Make it easier to go quickly to a specific commit
  gitk: Show the current view's name in the window title
  gitk: Add mouse right-click options to copy path and branch name
  gitk: Remove mc parameter from proc show_error
  gitk: Fix error when changing colors after closing "List references" window
  gitk: Replace catch {unset foo} with unset -nocomplain foo
  gitk: Rearrange window title to be more conventional
  gitk: sv.po: Update Swedish translation (305t0f0u)
  gitk: Fix bad English grammar "Matches none Commit Info"

Merge branch 'jk/pack-protocol-doc'

Streamline documentation of the pkt-line protocol.

* jk/pack-protocol-doc:
  pack-protocol: clarify LF-handling in PKT-LINE()

Merge branch 'mp/t7060-diff-index-test'

Fix an old test that was doing the same thing as another one.

* mp/t7060-diff-index-test:
  t7060: actually test "git diff-index --cached -M"

Merge branch 'gb/apply-comment-typofix'

* gb/apply-comment-typofix:
  apply: comment grammar fix

gitk: Accelerators for the main menu

This allows fast, keyboard-only usage of the menu (e.g. Alt+V, N to open a
new view).

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>

gitk: Adjust the menu line numbers to compensate for the new entry

Commit d835dbb9 ("gitk: Add a "Copy commit summary" command",
2015-08-13) in the upstream gitk repo added a new context menu entry.
Therefore, the line numbers of the entries below the new one need to be
adjusted when their text or state is changed.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>

Documentation/config: fix formatting for branch.*.rebase and pull.rebase

Don't format the second paragraph as a literal block.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tag, update-ref: improve description of option "create-reflog"

The description of option "create-reflog" is "create_reflog", which
is neither a good description, nor a sensible string to translate.
Change it to a more meaningful message.

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

pull: don't mark values for option "rebase" for translation

"false|true|preserve" are actual values for option "rebase"
of the "git-pull" command and should therefore not be marked
for translation.

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Makefile: use SHELL_PATH when running generate-cmdlist.sh

Non-POSIX shells, such as /bin/sh on SunOS, do not support $((...))
arithmetic expansion or $(...) command substitution needed by
generate-cmdlist.sh.  Make sure that we use a POSIX compliant shell
$(SHELL_PATH) when running generate-cmdlist.sh.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Acked-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>