aboutsummaryrefslogtreecommitdiff
path: root/http-push.c
Commit message (Collapse)AuthorAge
* Merge branch 'tg/memfixes'Junio C Hamano2017-10-07
|\ | | | | | | | | | | | | | | | | Fixes for a handful memory access issues identified by valgrind. * tg/memfixes: sub-process: use child_process.args instead of child_process.argv http-push: fix construction of hex value from path path.c: fix uninitialized memory access
| * http-push: fix construction of hex value from pathThomas Gummerer2017-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The get_oid_hex_from_objpath takes care of creating a oid from a pathname. It does this by memcpy'ing the first two bytes of the path to the "hex" string, then skipping the '/', and then copying the rest of the path to the "hex" string. Currently it fails to increase the pointer to the hex string, so the second memcpy invocation just mashes over what was copied in the first one, and leaves the last two bytes in the string uninitialized. This breaks valgrind in t5540, although the test passes without valgrind: ==5490== Use of uninitialised value of size 8 ==5490== at 0x13C6B5: hexval (cache.h:1238) ==5490== by 0x13C6DB: hex2chr (cache.h:1247) ==5490== by 0x13C734: get_sha1_hex (hex.c:42) ==5490== by 0x13C78E: get_oid_hex (hex.c:53) ==5490== by 0x118BDA: get_oid_hex_from_objpath (http-push.c:1023) ==5490== by 0x118C92: process_ls_object (http-push.c:1038) ==5490== by 0x118E5B: handle_remote_ls_ctx (http-push.c:1077) ==5490== by 0x118227: xml_end_tag (http-push.c:815) ==5490== by 0x50C1448: ??? (in /usr/lib/libexpat.so.1.6.6) ==5490== by 0x50C221B: ??? (in /usr/lib/libexpat.so.1.6.6) ==5490== by 0x50BFBF2: ??? (in /usr/lib/libexpat.so.1.6.6) ==5490== by 0x50C0B24: ??? (in /usr/lib/libexpat.so.1.6.6) ==5490== Uninitialised value was created by a stack allocation ==5490== at 0x118B63: get_oid_hex_from_objpath (http-push.c:1012) ==5490== Fix this by correctly incrementing the pointer to the "hex" variable, so the first two bytes are left untouched by the memcpy call, and the last two bytes are correctly initialized. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | consistently use "fallthrough" comments in switchesJeff King2017-09-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Gcc 7 adds -Wimplicit-fallthrough, which can warn when a switch case falls through to the next case. The general idea is that the compiler can't tell if this was intentional or not, so you should annotate any intentional fall-throughs as such, leaving it to complain about any unannotated ones. There's a GNU __attribute__ which can be used for annotation, but of course we'd have to #ifdef it away on non-gcc compilers. Gcc will also recognize specially-formatted comments, which matches our current practice. Let's extend that practice to all of the unannotated sites (which I did look over and verify that they were behaving as intended). Ideally in each case we'd actually give some reasons in the comment about why we're falling through, or what we're falling through to. And gcc does support that with -Wimplicit-fallthrough=2, which relaxes the comment pattern matching to anything that contains "fallthrough" (or a variety of spelling variants). However, this isn't the default for -Wimplicit-fallthrough, nor for -Wextra. In the name of simplicity, it's probably better for us to support the default level, which requires "fallthrough" to be the only thing in the comment (modulo some window dressing like "else" and some punctuation; see the gcc manual for the complete set of patterns). This patch suppresses all warnings due to -Wimplicit-fallthrough. We might eventually want to add that to the DEVELOPER Makefile knob, but we should probably wait until gcc 7 is more widely adopted (since earlier versions will complain about the unknown warning type). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack: move find_sha1_pack()Jonathan Tan2017-08-23
|/ | | | | Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* coccinelle: make use of the "type" FREE_AND_NULL() ruleÆvar Arnfjörð Bjarmason2017-06-16
| | | | | | | | | | Apply the result of the just-added coccinelle rule. This manually excludes a few occurrences, mostly things that resulted in many FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent change. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* object: convert parse_object* to take struct object_idbrian m. carlson2017-05-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make parse_object, parse_object_or_die, and parse_object_buffer take a pointer to struct object_id. Remove the temporary variables inserted earlier, since they are no longer necessary. Transform all of the callers using the following semantic patch: @@ expression E1; @@ - parse_object(E1.hash) + parse_object(&E1) @@ expression E1; @@ - parse_object(E1->hash) + parse_object(E1) @@ expression E1, E2; @@ - parse_object_or_die(E1.hash, E2) + parse_object_or_die(&E1, E2) @@ expression E1, E2; @@ - parse_object_or_die(E1->hash, E2) + parse_object_or_die(E1, E2) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1.hash, E2, E3, E4, E5) + parse_object_buffer(&E1, E2, E3, E4, E5) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1->hash, E2, E3, E4, E5) + parse_object_buffer(E1, E2, E3, E4, E5) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* http-push: convert process_ls_object and descendants to object_idbrian m. carlson2017-05-08
| | | | | | | | | | | | Rename one function to reflect that it now uses struct object_id. This conversion is a prerequisite for converting parse_object. Note that while the use of a buffer that is exactly forty bytes long looks questionable, get_oid_hex reads exactly the right number of bytes and does not require the data to be NUL-terminated. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Convert lookup_tree to struct object_idbrian m. carlson2017-05-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | Convert the lookup_tree function to take a pointer to struct object_id. The commit was created with manual changes to tree.c, tree.h, and object.c, plus the following semantic patch: @@ @@ - lookup_tree(EMPTY_TREE_SHA1_BIN) + lookup_tree(&empty_tree_oid) @@ expression E1; @@ - lookup_tree(E1.hash) + lookup_tree(&E1) @@ expression E1; @@ - lookup_tree(E1->hash) + lookup_tree(E1) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Convert lookup_blob to struct object_idbrian m. carlson2017-05-08
| | | | | | | | | | | | | | | | | | | | | | Convert lookup_blob to take a pointer to struct object_id. The commit was created with manual changes to blob.c and blob.h, plus the following semantic patch: @@ expression E1; @@ - lookup_blob(E1.hash) + lookup_blob(&E1) @@ expression E1; @@ - lookup_blob(E1->hash) + lookup_blob(E1) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Convert lookup_commit* to struct object_idbrian m. carlson2017-05-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert lookup_commit, lookup_commit_or_die, lookup_commit_reference, and lookup_commit_reference_gently to take struct object_id arguments. Introduce a temporary in parse_object buffer in order to convert this function. This is required since in order to convert parse_object and parse_object_buffer, lookup_commit_reference_gently and lookup_commit_or_die would need to be converted. Not introducing a temporary would therefore require that lookup_commit_or_die take a struct object_id *, but lookup_commit would take unsigned char *, leaving a confusing and hard-to-use interface. parse_object_buffer will lose this temporary in a later patch. This commit was created with manual changes to commit.c, commit.h, and object.c, plus the following semantic patch: @@ expression E1, E2; @@ - lookup_commit_reference_gently(E1.hash, E2) + lookup_commit_reference_gently(&E1, E2) @@ expression E1, E2; @@ - lookup_commit_reference_gently(E1->hash, E2) + lookup_commit_reference_gently(E1, E2) @@ expression E1; @@ - lookup_commit_reference(E1.hash) + lookup_commit_reference(&E1) @@ expression E1; @@ - lookup_commit_reference(E1->hash) + lookup_commit_reference(E1) @@ expression E1; @@ - lookup_commit(E1.hash) + lookup_commit(&E1) @@ expression E1; @@ - lookup_commit(E1->hash) + lookup_commit(E1) @@ expression E1, E2; @@ - lookup_commit_or_die(E1.hash, E2) + lookup_commit_or_die(&E1, E2) @@ expression E1, E2; @@ - lookup_commit_or_die(E1->hash, E2) + lookup_commit_or_die(E1, E2) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* http-push: convert some static functions to struct object_idbrian m. carlson2017-05-08
| | | | | | | | Among the functions converted is a caller of lookup_commit_or_die, which we will convert later on. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* http-push: don't check return value of lookup_unknown_object()René Scharfe2017-03-18
| | | | | | | | This function always returns a reference to an object, creating one if needed, so remove the unnecessary NULL check. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jk/common-main' into maintJunio C Hamano2016-09-08
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are certain house-keeping tasks that need to be performed at the very beginning of any Git program, and programs that are not built-in commands had to do them exactly the same way as "git" potty does. It was easy to make mistakes in one-off standalone programs (like test helpers). A common "main()" function that calls cmd_main() of individual program has been introduced to make it harder to make mistakes. * jk/common-main: mingw: declare main()'s argv as const common-main: call git_setup_gettext() common-main: call restore_sigpipe_to_default() common-main: call sanitize_stdfds() common-main: call git_extract_argv0_path() add an extra level of indirection to main()
| * Merge branch 'jk/common-main-2.8' into jk/common-mainJunio C Hamano2016-07-06
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/common-main-2.8: mingw: declare main()'s argv as const common-main: call git_setup_gettext() common-main: call restore_sigpipe_to_default() common-main: call sanitize_stdfds() common-main: call git_extract_argv0_path() add an extra level of indirection to main()
| | * common-main: call git_setup_gettext()Jeff King2016-07-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This should be part of every program, as otherwise users do not get translated error messages. However, some external commands forgot to do so (e.g., git-credential-store). This fixes them, and eliminates the repeated code in programs that did remember to use it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | * common-main: call git_extract_argv0_path()Jeff King2016-07-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Every program which links against libgit.a must call this function, or risk hitting an assert() in system_path() that checks whether we have configured argv0_path (though only when RUNTIME_PREFIX is defined, so essentially only on Windows). Looking at the diff, you can see that putting it into the common main() saves us having to do it individually in each of the external commands. But what you can't see are the cases where we _should_ have been doing so, but weren't (e.g., git-credential-store, and all of the t/helper test programs). This has been an accident-waiting-to-happen for a long time, but wasn't triggered until recently because it involves one of those programs actually calling system_path(). That happened with git-credential-store in v2.8.0 with ae5f677 (lazily load core.sharedrepository, 2016-03-11). The program: - takes a lock file, which... - opens a tempfile, which... - calls adjust_shared_perm to fix permissions, which... - lazy-loads the config (as of ae5f677), which... - calls system_path() to find the location of /etc/gitconfig On systems with RUNTIME_PREFIX, this means credential-store reliably hits that assert() and cannot be used. We never noticed in the test suite, because we set GIT_CONFIG_NOSYSTEM there, which skips the system_path() lookup entirely. But if we were to tweak git_config() to find /etc/gitconfig even when we aren't going to open it, then the test suite shows multiple failures (for credential-store, and for some other test helpers). I didn't include that tweak here because it's way too specific to this particular call to be worth carrying around what is essentially dead code. The implementation is fairly straightforward, with one exception: there is exactly one caller (git.c) that actually cares about the result of the function, and not the side-effect of setting up argv0_path. We can accommodate that by simply replacing the value of argv[0] in the array we hand down to cmd_main(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | * add an extra level of indirection to main()Jeff King2016-07-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are certain startup tasks that we expect every git process to do. In some cases this is just to improve the quality of the program (e.g., setting up gettext()). In others it is a requirement for using certain functions in libgit.a (e.g., system_path() expects that you have called git_extract_argv0_path()). Most commands are builtins and are covered by the git.c version of main(). However, there are still a few external commands that use their own main(). Each of these has to remember to include the correct startup sequence, and we are not always consistent. Rather than just fix the inconsistencies, let's make this harder to get wrong by providing a common main() that can run this standard startup. We basically have two options to do this: - the compat/mingw.h file already does something like this by adding a #define that replaces the definition of main with a wrapper that calls mingw_startup(). The upside is that the code in each program doesn't need to be changed at all; it's rewritten on the fly by the preprocessor. The downside is that it may make debugging of the startup sequence a bit more confusing, as the preprocessor is quietly inserting new code. - the builtin functions are all of the form cmd_foo(), and git.c's main() calls them. This is much more explicit, which may make things more obvious to somebody reading the code. It's also more flexible (because of course we have to figure out _which_ cmd_foo() to call). The downside is that each of the builtins must define cmd_foo(), instead of just main(). This patch chooses the latter option, preferring the more explicit approach, even though it is more invasive. We introduce a new file common-main.c, with the "real" main. It expects to call cmd_main() from whatever other objects it is linked against. We link common-main.o against anything that links against libgit.a, since we know that such programs will need to do this setup. Note that common-main.o can't actually go inside libgit.a, as the linker would not pick up its main() function automatically (it has no callers). The rest of the patch is just adjusting all of the various external programs (mostly in t/helper) to use cmd_main(). I've provided a global declaration for cmd_main(), which means that all of the programs also need to match its signature. In particular, many functions need to switch to "const char **" instead of "char **" for argv. This effect ripples out to a few other variables and functions, as well. This makes the patch even more invasive, but the end result is much better. We should be treating argv strings as const anyway, and now all programs conform to the same signature (which also matches the way builtins are defined). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | use strbuf_addstr() for adding constant strings to a strbufRené Scharfe2016-08-01
|/ / | | | | | | | | | | | | | | | | | | | | | | | | Replace uses of strbuf_addf() for adding strings with more lightweight strbuf_addstr() calls. In http-push.c it becomes easier to see what's going on without having to verfiy that the definition of PROPFIND_ALL_REQUEST doesn't contain any format specifiers. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'bc/object-id'Junio C Hamano2016-05-06
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move from unsigned char[20] to struct object_id continues. * bc/object-id: match-trees: convert several leaf functions to use struct object_id tree-walk: convert tree_entry_extract() to use struct object_id struct name_entry: use struct object_id instead of unsigned char sha1[20] match-trees: convert shift_tree() and shift_tree_by() to use object_id test-match-trees: convert to use struct object_id sha1-name: introduce a get_oid() function
| * | struct name_entry: use struct object_id instead of unsigned char sha1[20]brian m. carlson2016-04-25
| |/ | | | | | | | | Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | http: support sending custom HTTP headersJohannes Schindelin2016-04-27
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We introduce a way to send custom HTTP headers with all requests. This allows us, for example, to send an extra token from build agents for temporary access to private repositories. (This is the use case that triggered this patch.) This feature can be used like this: git -c http.extraheader='Secret: sssh!' fetch $URL $REF Note that `curl_easy_setopt(..., CURLOPT_HTTPHEADER, ...)` takes only a single list, overriding any previous call. This means we have to collect _all_ of the headers we want to use into a single list, and feed it to cURL in one shot. Since we already unconditionally set a "pragma" header when initializing the curl handles, we can add our new headers to that list. For callers which override the default header list (like probe_rpc), we provide `http_copy_default_headers()` so they can do the same trick. Big thanks to Jeff King and Junio Hamano for their outstanding help and patient reviews. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* http-push: stop using name_pathJeff King2016-02-12
| | | | | | | | | | | | | | | | | | | | | | | The graph traversal code here passes along a name_path to build up the pathname at which we find each blob. But we never actually do anything with the resulting names, making it a waste of code and memory. This usage came in aa1dbc9 (Update http-push functionality, 2006-03-07), and originally the result was passed to "add_object" (which stored it, but didn't really use it, either). But we stopped using that function in 1f1e895 (Add "named object array" concept, 2006-06-19) in favor of storing just the objects themselves. Moreover, the generation of the name in process_tree() is buggy. It sticks "name" onto the end of the name_path linked list, and then passes it down again as it recurses (instead of "entry.path"). So it's a good thing this was unused, as the resulting path for "a/b/c/d" would end up as "a/a/a/a". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Remove get_object_hash.brian m. carlson2015-11-20
| | | | | | | | | Convert all instances of get_object_hash to use an appropriate reference to the hash member of the oid member of struct object. This provides no functional change, as it is essentially a macro substitution. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
* Convert struct object to object_idbrian m. carlson2015-11-20
| | | | | | | | | struct object is one of the major data structures dealing with object IDs. Convert it to use struct object_id instead of an unsigned char array. Convert get_object_hash to refer to the new member as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
* Add several uses of get_object_hash.brian m. carlson2015-11-20
| | | | | | | | | | | Convert most instances where the sha1 member of struct object is dereferenced to use get_object_hash. Most instances that are passed to functions that have versions taking struct object_id, such as get_sha1_hex/get_oid_hex, or instances that can be trivially converted to use struct object_id instead, are not converted. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
* ref_newer: convert to use struct object_idbrian m. carlson2015-11-20
| | | | | | | | Convert ref_newer and its caller to use struct object_id instead of unsigned char *. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
* Convert struct ref to use object_id.brian m. carlson2015-11-20
| | | | | | | | Use struct object_id in three fields in struct ref and convert all the necessary places that use it. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
* drop strcpy in favor of raw sha1_to_hexJeff King2015-10-05
| | | | | | | | | | | | | | | | 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>
* http-push: use an argv_array for setup_revisionsJeff King2015-10-05
| | | | | | | | | | | 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>
* http-push: use strbuf instead of fwrite_bufferJeff King2015-09-25
| | | | | | | | | | | | | | | 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>
* http-push: replace strcat with xsnprintfJeff King2015-09-25
| | | | | | | | | | | | | | 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>
* use xsnprintf for generating git object headersJeff King2015-09-25
| | | | | | | | | | | | | | | | | 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>
* convert trivial sprintf / strcpy calls to xsnprintfJeff King2015-09-25
| | | | | | | | | | | | | | | | | | 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>
* Merge branch 'sb/leaks'Junio C Hamano2015-03-27
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | * sb/leaks: http: release the memory of a http pack request as well read-cache: fix memleak add_to_index(): free unused cache-entry commit.c: fix a memory leak http-push: remove unneeded cleanup merge-recursive: fix memleaks merge-blobs.c: fix a memleak builtin/apply.c: fix a memleak update-index: fix a memleak read-cache: free cache entry in add_to_index in case of early return
| * http-push: remove unneeded cleanupStefan Beller2015-03-23
| | | | | | | | | | | | | | | | preq is NULL as the condition the line before dictates. And the cleanup function release_http_pack_request is not null pointer safe. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'rs/deflate-init-cleanup'Junio C Hamano2015-03-17
|\ \ | |/ |/| | | | | | | | | Code simplification. * rs/deflate-init-cleanup: zlib: initialize git_zstream in git_deflate_init{,_gzip,_raw}
| * zlib: initialize git_zstream in git_deflate_init{,_gzip,_raw}René Scharfe2015-03-05
| | | | | | | | | | | | | | | | Clear the git_zstream variable at the start of git_deflate_init() etc. so that callers don't have to do that. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Merge branch 'ah/fix-http-push' into maintJunio C Hamano2014-07-22
| |\ | | | | | | | | | | | | * ah/fix-http-push: http-push.c: make CURLOPT_IOCTLDATA a usable pointer
* | | http-push: trim trailing newline from remote symrefJeff King2015-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we fetch a symbolic ref file from the remote, we get the whole string "ref: refs/heads/master\n", recognize it by skipping past the "ref: ", and store the rest. We should chomp the trailing newline. This bug was introduced in ae021d8 (use skip_prefix to avoid magic numbers, 2014-06-18), which did not notice that the length computation fed to xmemdupz was quietly tweaked by 1 to account for this. We can solve it by explicitly trimming the newline, which is more obvious. Note that we use strbuf_rtrim here, which will actually cut off any trailing whitespace, not just a single newline. This is a good thing, though, as it makes our parsing more liberal (and spaces are not valid in refnames anyway). Signed-off-by: Jeff King <peff@peff.net> Tested-by: Kyle J. McKay <mackyle@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'ah/fix-http-push'Junio C Hamano2014-07-16
|\ \ \ | | |/ | |/| | | | | | | | | | | | | | | | An ancient rewrite passed a wrong pointer to a curl library function in a rarely used code path. * ah/fix-http-push: http-push.c: make CURLOPT_IOCTLDATA a usable pointer
| * | http-push.c: make CURLOPT_IOCTLDATA a usable pointerAbbaad Haider2014-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes a small bug affecting push to remotes which use some sort of multi-pass authentication. In particular the bug affected SabreDAV as configured by Box.com [1]. It must be a weird server configuration for the bug to have survived this long. Someone should write a test for it. [1] http://marc.info/?l=git&m=140460482604482 Signed-off-by: Abbaad Haider <abbaad@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jk/xstrfmt'Junio C Hamano2014-07-09
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/xstrfmt: setup_git_env(): introduce git_path_from_env() helper unique_path: fix unlikely heap overflow walker_fetch: fix minor memory leak merge: use argv_array when spawning merge strategy sequencer: use argv_array_pushf setup_git_env: use git_pathdup instead of xmalloc + sprintf use xstrfmt to replace xmalloc + strcpy/strcat use xstrfmt to replace xmalloc + sprintf use xstrdup instead of xmalloc + strcpy use xstrfmt in favor of manual size calculations strbuf: add xstrfmt helper
| * | | use xstrfmt to replace xmalloc + sprintfJeff King2014-06-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is one line shorter, and makes sure the length in the malloc and sprintf steps match. These conversions are very straightforward; we can drop the malloc entirely, and replace the sprintf with xstrfmt. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | use xstrdup instead of xmalloc + strcpyJeff King2014-06-19
| | |/ | |/| | | | | | | | | | | | | | | | | | | This is one line shorter, and makes sure the length in the malloc and copy steps match. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | http-push: refactor parsing of remote object namesJeff King2014-06-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We get loose object names like "objects/??/..." from the remote side, and need to convert them to their hex representation. The code to do so is rather hard to follow, as it uses some calculated lengths whose origins are hard to understand and verify (e.g., the path must be exactly 49 characters long. why? Why doesn't the strcpy overflow obj_hex, which is the same length as path?). We can simplify this a bit by using skip_prefix, using standard 40- and 20-character buffers for hex and binary sha1s, and adding some comments. We also drop a totally bogus comment that claims strlcpy cannot be used because "path" is not NUL-terminated. Right between a call to strlen(path) and strcpy(path). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | use skip_prefix to avoid magic numbersJeff King2014-06-20
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | http-push.c: rearrange xcalloc argumentsBrian Gesiak2014-05-27
| | | | | | | | | | | | | | | | | | | | | | xcalloc() takes two arguments: the number of elements and their size. http-push passes the arguments in reverse order, passing the size of a repo, followed by the number to allocate. Rearrange them so they are in the correct order. Signed-off-by: Brian Gesiak <modocache@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | object.h: centralize object flag allocationNguyễn Thái Ngọc Duy2014-03-25
| | | | | | | | | | | | | | | | | | | | While the field "flags" is mainly used by the revision walker, it is also used in many other places. Centralize the whole flag allocation to one place for a better overview (and easier to move flags if we have too). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | replace {pre,suf}fixcmp() with {starts,ends}_with()Christian Couder2013-12-05
|/ | | | | | | | | | | | | | | | | | | | | | | Leaving only the function definitions and declarations so that any new topic in flight can still make use of the old functions, replace existing uses of the prefixcmp() and suffixcmp() with new API functions. The change can be recreated by mechanically applying this: $ git grep -l -e prefixcmp -e suffixcmp -- \*.c | grep -v strbuf\\.c | xargs perl -pi -e ' s|!prefixcmp\(|starts_with\(|g; s|prefixcmp\(|!starts_with\(|g; s|!suffixcmp\(|ends_with\(|g; s|suffixcmp\(|!ends_with\(|g; ' on the result of preparatory changes in this series. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jk/http-auth-redirects'Junio C Hamano2013-10-30
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Handle the case where http transport gets redirected during the authorization request better. * jk/http-auth-redirects: http.c: Spell the null pointer as NULL remote-curl: rewrite base url from info/refs redirects remote-curl: store url as a strbuf remote-curl: make refs_url a strbuf http: update base URLs when we see redirects http: provide effective url to callers http: hoist credential request out of handle_curl_result http: refactor options to http_get_* http_request: factor out curlinfo_strbuf http_get_file: style fixes