kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Martin Ågren	e7347cb9ba	config: let `config_store_data_clear()` handle `key` Instead of remembering to free `key` in each code path, let `config_store_data_clear()` handle that. We still need to free it before replacing it, though. Move that freeing closer to the replacing to be safe. Note that in that same part of the code, we can no longer set `key` to the original pointer, but need to `xstrdup()` it. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	3b82542dff	config: let `config_store_data_clear()` handle `value_regex` Instead of duplicating the logic for clearing up `value_regex`, let `config_store_data_clear()` handle that. When `regcomp()` fails, the current code does not call `regfree()`. Make sure we do the same by immediately invalidating `value_regex`. Some implementations are able to handle such an extra `regfree()`-call [1], but from the example in [2], we should not do so. (The language itself in [2] is not super-clear on this.) [1] https://www.redhat.com/archives/libvir-list/2013-September/msg00262.html [2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html Researched-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	2a00e594e5	config: free resources of `struct config_store_data` Commit `fee8572c6d` (config: avoid using the global variable `store`, 2018-04-09) dropped the staticness of a certain struct, instead letting the users create an instance on the stack and pass around a pointer. We do not free all the memory that the struct tracks. When the struct was static, the memory would always be reachable. Now that we keep the struct on the stack, though, as soon as we return, it goes out of scope and we leak the memory it points to. In particular, we leak the memory pointed to by the `parsed` and `seen` fields. Introduce and use a helper function `config_store_data_clear()` to plug these leaks. The memory tracked here is config parser events. Once the users (`git_config_set_multivar_in_file_gently()` and `git_config_copy_or_rename_section_in_file()` at the moment) are done, no-one should be holding on to a pointer into this memory. There are two more members of the struct that are candidates for freeing in this new function (`key` and `value_regex`). Those are actually already being taken care of. The next couple of patches will move their freeing into the function we are adding here. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	438a87d1e2	config: a user-provided invalid section is not a BUG This was pointed out by Jeff King while the empty-config-section-fix patch series was cooking, and was not addressed in time for that patch series to advance to `master`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	033abf97fc	Replace all die("BUG: ...") calls by BUG() ones In `d8193743e0` (usage.c: add BUG() function, 2017-05-12), a new macro was introduced to use for reporting bugs instead of die(). It was then subsequently used to convert one single caller in `588a538ae5` (setup_git_env: convert die("BUG") to BUG(), 2017-05-12). The cover letter of the patch series containing this patch (cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not terribly clear why only one call site was converted, or what the plan is for other, similar calls to die() to report bugs. Let's just convert all remaining ones in one fell swoop. This trick was performed by this invocation: sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ryan Dammrose	960786e761	push: colorize errors This is an attempt to resolve an issue I experience with people that are new to Git -- especially colleagues in a team setting -- where they miss that their push to a remote location failed because the failure and success both return a block of white text. An example is if I push something to a remote repository and then a colleague attempts to push to the same remote repository and the push fails because it requires them to pull first, but they don't notice because a success and failure both return a block of white text. They then continue about their business, thinking it has been successfully pushed. This patch colorizes the errors and hints (in red and yellow, respectively) so whenever there is a failure when pushing to a remote repository that fails, it is more noticeable. [jes: fixed a couple bugs, added the color.{advice,push,transport} settings, refactored to use want_color_stderr().] Signed-off-by: Ryan Dammrose ryandammrose@gmail.com Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Taylor Blau	6d2f9acc0f	config.c: introduce 'git_config_color' to parse ANSI colors In preparation for adding `--type=color` to the `git-config(1)` builtin, let's introduce a color parsing utility, `git_config_color` in a similar fashion to `git_config_<type>`. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Lars Schneider	e92d622536	convert: add round trip check based on 'core.checkRoundtripEncoding' UTF supports lossless conversion round tripping and conversions between UTF and other encodings are mostly round trip safe as Unicode aims to be a superset of all other character encodings. However, certain encodings (e.g. SHIFT-JIS) are known to have round trip issues [1]. Add 'core.checkRoundtripEncoding', which contains a comma separated list of encodings, to define for what encodings Git should check the conversion round trip if they are used in the 'working-tree-encoding' attribute. Set SHIFT-JIS as default value for 'core.checkRoundtripEncoding'. [1] https://support.microsoft.com/en-us/help/170559/prb-conversion-problem-between-shift-jis-and-unicode Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Stefan Beller	d807c4a01d	exec_cmd: rename to use dash in file name This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller <sbeller@google.com>	7 years ago
Derrick Stolee	1b70dfd594	commit-graph: add core.commitGraph setting The commit graph feature is controlled by the new core.commitGraph config setting. This defaults to 0, so the feature is opt-in. The intention of core.commitGraph is that a user can always stop checking for or parsing commit graph files if core.commitGraph=0. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	c71d8bb38a	git_config_set: reuse empty sections It can happen quite easily that the last setting in a config section is removed, and to avoid confusion when there are comments in the config about that section, we keep a lone section header, i.e. an empty section. Now that we use the `event_fn` callback, it is easy to add support for re-using empty sections, so let's do that. Note: t5512-ls-remote requires that this change is applied after the patch "git config --unset: remove empty sections (in the common case)": without that patch, there would be empty `transfer` and `uploadpack` sections ready for reuse, but in the wrong order (and sconsequently, t5512's "overrides work between mixed transfer/upload-pack hideRefs" would fail). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	22aedfccd0	git config --unset: remove empty sections (in the common case) The original reasoning for not removing section headers upon removal of the last entry went like this: the user could have added comments about the section, or about the entries therein, and if there were other comments there, we would not know whether we should remove them. In particular, a concocted example was presented that looked like this (and was added to t1300): # some generic comment on the configuration file itself # a comment specific to this "section" section. [section] # some intervening lines # that should also be dropped key = value # please be careful when you update the above variable The ideal thing for `git config --unset section.key` in this case would be to leave only the first line behind, because all the other comments are now obsolete. However, this is unfeasible, short of adding a complete Natural Language Processing module to Git, which seems not only a lot of work, but a totally unreasonable feature (for little benefit to most users). Now, the real kicker about this problem is: most users do not edit their config files at all! In their use case, the config looks like this instead: [section] key = value ... and it is totally obvious what should happen if the entry is removed: the entire section should vanish. Let's generalize this observation to this conservative strategy: if we are removing the last entry from a section, and there are no comments inside that section nor surrounding it, then remove the entire section. Otherwise behave as before: leave the now-empty section (including those comments, even ones about the now-deleted entry). We have to be extra careful to handle the case where more than one entry is removed: any subset of them might be the last entries of their respective sections (and if there are no comments in or around that section, the section should be removed, too). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	6ae996f2ac	git_config_set: make use of the config parser's event stream In the recent commit with the title "config: introduce an optional event stream while parsing", we introduced an optional callback to keep track of the config parser's events "comment", "white-space", "section header" and "entry". One motivation for this feature was to make use of it in the code that edits the config. And this commit makes it so. Note: this patch changes the meaning of the `seen` array that records whether we saw the config entry that is to be edited: previously, it contained the end offset of the found entry. Now, we introduce a new array `parsed` that keeps a record of all config parser events (with begin/end offsets), and the items in the `seen` array now point into the `parsed` array. There are two reasons why we do it this way: 1. To keep the implementation simple, the config parser's event stream reports the event only after the config callback was called, so we would not receive the begin offset otherwise. 2. In the following patches, we will re-use the `parsed` array to fix two long-standing bugs related to empty sections. Note that this also makes the code more robust with respect to finding the begin offset of the part(s) of the config file to be edited, as we no longer back-track to find the beginning of the line. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	5221c3159f	git_config_set: do not use a state machine While a neat theoretical construct, state machines are hard to read. In this instance, it does not even make a whole lot of sense because we are more interested in flags, anyway: has the section been seen? Has the key been seen? Does the current section match the key we are looking for? Besides, the state `SECTION_SEEN` was named in a misleading way: it did not indicate that we saw the section matching the key we are looking for, but it instead indicated that we are currently in that section. Let's just replace the state machine logic by clear and obvious flags. This will also make it easier to review the upcoming patches to use the newly-introduced `event_fn` callback of the config parser. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	668b9ade6b	config_set_store: rename some fields for consistency The `seen` field is the actual length of the `offset` array, and the `offset_alloc` field records what was allocated (to avoid resizing wherever `seen` has to be incremented). Elsewhere, we use the convention `name` for the array, where `name` is descriptive enough to guess its purpose, `name_nr` for the actual length and `name_alloc` to record the maximum length without needing to resize. Let's make the names of the fields in question consistent with that convention. This will also help with the next steps where we will let the git_config_set() machinery use the config event stream that we just introduced. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	fee8572c6d	config: avoid using the global variable `store` It is much easier to reason about, when the config code to set/unset variables or to remove/rename sections does not rely on a global (or file-local) variable. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	8032cc4462	config: introduce an optional event stream while parsing This extends our config parser so that it can optionally produce an event stream via callback function, where it reports e.g. when a comment was parsed, or a section header, etc. This parser will be used subsequently to handle the scenarios better where removing config entries would make sections empty, or where a new entry could be added to an already-existing, empty section. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	46fc89ce74	config --replace-all: avoid extra line breaks When replacing multiple config entries at once, we did not re-set the flag that indicates whether we need to insert a new-line before the new entry. As a consequence, an extra new-line was inserted under certain circumstances. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Johannes Schindelin	83b7fd8771	git_config_set: fix off-by-two Currently, we are slightly overzealous When removing an entry from a config file of this form: [abc]a [xyz] key = value When calling `git config --unset abc.a` on this file, it leaves this (invalid) config behind: [ [xyz] key = value The reason is that we try to search for the beginning of the line (or for the end of the preceding section header on the same line) that defines abc.a, but as an optimization, we subtract 2 from the offset pointing just after the definition before we call find_beginning_of_line(). That function, however, also performs that optimization and promptly fails to find the section header correctly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	05e293c1ac	config: move flockfile() closer to unlocked functions Commit `260d408e32` (config: use getc_unlocked when reading from file, 2015-04-16) taught git_config_from_file() to lock the filehandle so that we could safely use the faster unlocked functions to access the handle. However, it split the logic into two places: 1. The master lock/unlock happens in git_config_from_file(). 2. The decision to use the unlocked functions happens in do_config_from_file(). That means that if anybody calls the latter function, they will accidentally use the unlocked functions without holding the lock. And indeed, git_config_from_stdin() does so. In practice, this hasn't been a problem since this code isn't generally multi-threaded (and even if some Git program happened to have another thread running, it's unlikely to be reading from stdin). But it's a good practice to make sure we're always holding the lock before using the unlocked functions. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
brian m. carlson	b4f5aca40e	sha1_file: convert read_sha1_file to struct object_id Convert read_sha1_file to take a pointer to struct object_id and rename it read_object_file. Do the same for read_sha1_file_extended. Convert one use in grep.c to use the new function without any other code change, since the pointer being passed is a void pointer that is already initialized with a pointer to struct object_id. Update the declaration and definitions of the modified functions, and apply the following semantic patch to convert the remaining callers: @@ expression E1, E2, E3; @@ - read_sha1_file(E1.hash, E2, E3) + read_object_file(&E1, E2, E3) @@ expression E1, E2, E3; @@ - read_sha1_file(E1->hash, E2, E3) + read_object_file(E1, E2, E3) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1.hash, E2, E3, E4) + read_object_file_extended(&E1, E2, E3, E4) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1->hash, E2, E3, E4) + read_object_file_extended(E1, E2, E3, E4) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Torsten Bögershausen	8462ff43e4	convert_to_git(): safe_crlf/checksafe becomes int conv_flags When calling convert_to_git(), the checksafe parameter defined what should happen if the EOL conversion (CRLF --> LF --> CRLF) does not roundtrip cleanly. In addition, it also defined if line endings should be renormalized (CRLF --> LF) or kept as they are. checksafe was an safe_crlf enum with these values: SAFE_CRLF_FALSE: do nothing in case of EOL roundtrip errors SAFE_CRLF_FAIL: die in case of EOL roundtrip errors SAFE_CRLF_WARN: print a warning in case of EOL roundtrip errors SAFE_CRLF_RENORMALIZE: change CRLF to LF SAFE_CRLF_KEEP_CRLF: keep all line endings as they are In some cases the integer value 0 was passed as checksafe parameter instead of the correct enum value SAFE_CRLF_FALSE. That was no problem because SAFE_CRLF_FALSE is defined as 0. FALSE/FAIL/WARN are different from RENORMALIZE and KEEP_CRLF. Therefore, an enum is not ideal. Let's use a integer bit pattern instead and rename the parameter to conv_flags to make it more generically usable. This allows us to extend the bit pattern in a subsequent commit. Reported-By: Randall S. Becker <rsbecker@nexbridge.com> Helped-By: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff Hostetler	1e1e39b308	partial-clone: define partial clone settings in config Create get and set routines for "partial clone" config settings. These will be used in a future commit by clone and fetch to remember the promisor remote and the default filter-spec. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
René Scharfe	782c030ea2	config: flip return value of write_section() `d9bd4cbb9c` (config: flip return value of store_write_*()) made write_section() follow the convention of write(2) to return -1 on error and the number of written bytes on success. `3b48045c6c` (Merge branch 'sd/branch-copy') changed it back to returning 0 on error and 1 on success, but left its callers still checking for negative values. Let write_section() follow the convention of write(2) again to meet the expectations of its callers. Reported-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>	7 years ago
Haaris Mehmood	5f9674243d	config: add --expiry-date Add --expiry-date as a data-type for config files when 'git config --get' is used. This will return any relative or fixed dates from config files as timestamps. This is useful for scripts (e.g. gc.reflogexpire) that work with timestamps so that '2.weeks' can be converted to a format acceptable by those scripts/functions. Following the convention of git_config_pathname(), move the helper function required for this feature from builtin/reflog.c to builtin/config.c where other similar functions exist (e.g. for --bool or --path), and match the order of parameters with other functions (i.e. output pointer as first parameter). Signed-off-by: Haaris Mehmood <hsed@unimetic.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Phillip Wood	c5e3bc6ec4	config: avoid "write_in_full(fd, buf, len) != len" pattern As explained in commit `06f46f237` (avoid "write_in_full(fd, buf, len) != len" pattern, 2017–09–13) the return value of write_in_full() is either -1 or the requested number of bytes. As such comparing the return value to an unsigned value such as strbuf.len will fail to catch errors. Change the code to use the preferred '< 0' check. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	33c643bb08	Revert "color: check color.ui in git_default_config()" This reverts commit `136c8c8b8f`. That commit was trying to address a bug caused by `4c7f1819b3` (make color.ui default to 'auto', 2013-06-10), in which plumbing like diff-tree defaulted to "auto" color, but did not respect a "color.ui" directive to disable it. But it also meant that we started respecting "color.ui" set to "always". This was a known problem, but `4c7f1819b3` argued that nobody ought to be doing that. However, that turned out to be wrong, and we got a number of bug reports related to "add -p" regressing in v2.14.2. Let's revert `136c8c8b8`, fixing the regression to "add -p". This leaves the problem from `4c7f1819b3` unfixed, but: 1. It's a pretty obscure problem in the first place. I only noticed it while working on the color code, and we haven't got a single bug report or complaint about it. 2. We can make a more moderate fix on top by respecting "never" but not "always" for plumbing commands. This is just the minimal fix to go back to the working state we had before v2.14.2. Note that this isn't a pure revert. We now have a test in t3701 which shows off the "add -p" regression. This can be flipped to success. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	837e34eba4	treewide: prefer lockfiles on the stack There is no longer any need to allocate and leak a `struct lock_file`. The previous patch addressed an instance where we needed a minor tweak alongside the trivial changes. Deal with the remaining instances where we allocate and leak a struct within a single function. Change them to have the `struct lock_file` on the stack instead. These instances were identified by running `git grep "^\sstruct lock_file\s\*"`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ben Peart	883e248b8a	fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files. When the index is read from disk, the fsmonitor index extension is used to flag the last known potentially dirty index entries. The registered core.fsmonitor command is called with the time the index was last updated and returns the list of files changed since that time. This list is used to flag any additional dirty cache entries and untracked cache directories. We can then use this valid state to speed up preload_index(), ie_match_stat(), and refresh_cache_ent() as they do not need to lstat() files to detect potential changes for those entries marked CE_FSMONITOR_VALID. In addition, if the untracked cache is turned on valid_cached_dir() can skip checking directories for new or changed files as fsmonitor will invalidate the cache only for those directories that have been identified as having potential changes. To keep the CE_FSMONITOR_VALID state accurate during git operations; when git updates a cache entry to match the current state on disk, it will now set the CE_FSMONITOR_VALID bit. Inversely, anytime git changes a cache entry, the CE_FSMONITOR_VALID bit is cleared and the corresponding untracked cache directory is marked invalid. Signed-off-by: Ben Peart <benpeart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ramsay Jones	071bcaab64	ALLOC_GROW: avoid -Wsign-compare warnings Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	1cf01a34ea	consistently use "fallthrough" comments in switches 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>	8 years ago
Jeff King	d9bd4cbb9c	config: flip return value of store_write_*() The store_write_section() and store_write_pairs() functions are basically high-level wrappers around write(). But their return values are flipped from our usual convention, using "1" for success and "0" for failure. Let's flip them to follow the usual write() conventions and update all callers. As these are local to config.c, it's unlikely that we'd have new callers in any topics in flight (which would be silently broken by our change). But just to be on the safe side, let's rename them to just write_section() and write_pairs(). That also accentuates their relationship with write(). Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	06f46f237a	avoid "write_in_full(fd, buf, len) != len" pattern The return value of write_in_full() is either "-1", or the requested number of bytes[1]. If we make a partial write before seeing an error, we still return -1, not a partial value. This goes back to `f6aa66cb95` (write_in_full: really write in full or return error on disk full., 2007-01-11). So checking anything except "was the return value negative" is pointless. And there are a couple of reasons not to do so: 1. It can do a funny signed/unsigned comparison. If your "len" is signed (e.g., a size_t) then the compiler will promote the "-1" to its unsigned variant. This works out for "!= len" (unless you really were trying to write the maximum size_t bytes), but is a bug if you check "< len" (an example of which was fixed recently in config.c). We should avoid promoting the mental model that you need to check the length at all, so that new sites are not tempted to copy us. 2. Checking for a negative value is shorter to type, especially when the length is an expression. 3. Linus says so. In `d34cf19b89` (Clean up write_in_full() users, 2007-01-11), right after the write_in_full() semantics were changed, he wrote: I really wish every "write_in_full()" user would just check against "<0" now, but this fixes the nasty and stupid ones. Appeals to authority aside, this makes it clear that writing it this way does not have an intentional benefit. It's a historical curiosity that we never bothered to clean up (and which was undoubtedly cargo-culted into new sites). So let's convert these obviously-correct cases (this includes write_str_in_full(), which is just a wrapper for write_in_full()). [1] A careful reader may notice there is one way that write_in_full() can return a different value. If we ask write() to write N bytes and get a return value that is _larger_ than N, we could return a larger total. But besides the fact that this would imply a totally broken version of write(), it would already invoke undefined behavior. Our internal remaining counter is an unsigned size_t, which means that subtracting too many byte will wrap it around to a very large number. So we'll instantly begin reading off the end of the buffer, trying to write gigabytes (or petabytes) of data. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	efacf609c8	config: avoid "write_in_full(fd, buf, len) < len" pattern The return type of write_in_full() is a signed ssize_t, because we may return "-1" on failure (even if we succeeded in writing some bytes). But "len" itself is may be an unsigned type (the function takes a size_t, but of course we may have something else in the calling function). So while it seems like: if (write_in_full(fd, buf, len) < len) die_errno("write error"); would trigger on error, it won't if "len" is unsigned. The compiler sees a signed/unsigned comparison and promotes the signed value, resulting in (size_t)-1, the highest possible size_t (or again, whatever type the caller has). This cannot possibly be smaller than "len", and so the conditional can never trigger. I scoured the code base for cases of this, but it turns out that these two in git_config_set_multivar_in_file_gently() are the only ones. Here our "len" is the difference between two size_t variables, making the result an unsigned size_t. We can fix this by just checking for a negative return value directly, as write_in_full() will never return any value except -1 or the full count. There's no addition to the test suite here, since you need to convince write() to fail in order to see the problem. The simplest reproduction recipe I came up with is to trigger ENOSPC: # make a limited-size filesystem dd if=/dev/zero of=small.disk bs=1M count=1 mke2fs small.disk mkdir mnt sudo mount -o loop small.disk mnt cd mnt sudo chown $USER:$USER . # make a config file with some content git config --file=config one.key value git config --file=config two.key value # now fill up the disk dd if=/dev/zero of=fill # and try to delete a key, which requires copying the rest # of the file to config.lock, and will fail on write() git config --file=config --unset two.key That final command should (and does after this patch) produce an error message due to the failed write, and leave the file intact. Instead, it silently ignores the failure and renames config.lock into place, leaving you with a totally empty config file! Reported-by: demerphq <demerphq@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Martin Ågren	d389028695	config: remove git_config_maybe_bool The function was deprecated in commit `89576613` ("treewide: deprecate git_config_maybe_bool, use git_parse_maybe_bool", 2017-08-07) and has no users. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	bfffb48c5d	stop leaking lock structs in some simple cases Now that it's safe to declare a "struct lock_file" on the stack, we can do so (and avoid an intentional leak). These leaks were found by running t0000 and t0001 under valgrind (though certainly other similar leaks exist and just don't happen to be exercised by those tests). Initializing the lock_file's inner tempfile with NULL is not strictly necessary in these cases, but it's a good practice to model. It means that if we were to call a function like rollback_lock_file() on a lock that was never taken in the first place, it becomes a quiet noop (rather than undefined behavior). Likewise, it's always safe to rollback_lock_file() on a file that has already been committed or deleted, since that operation is a noop on an inactive lockfile (and that's why the case in config.c can drop the "if (lock)" check as we move away from using a pointer). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	f991761eb8	config: use a static lock_file struct When modifying git config, we xcalloc() a struct lock_file but never free it. This is necessary because the tempfile code (upon which the locking code is built) requires that the resulting struct remain valid through the life of the program. However, it also confuses leak-checkers like valgrind because only the inner "struct tempfile" is still reachable; no pointer to the outer lock_file is kept. Other code paths solve this by using a single static lock struct. We can do the same here, because we know that we'll only lock and modify one config file at a time (and assertions within the lockfile code will ensure that this remains the case). That removes a real leak (when we fail to free the struct after locking fails) as well as removes the valgrind false positive. It also means that doing N sequential config-writes will use a constant amount of memory, rather than leaving stale lock_files for each. Note that since "lock" is no longer a pointer, it can't be NULL anymore. But that's OK. We used that feature only to avoid calling rollback_lock_file() on an already-committed lock. Since the lockfile code keeps its own "active" flag, it's a noop to rollback an inactive lock, and we don't have to worry about this ourselves. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Junio C Hamano	6e96cb5286	rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved These two configuration variables are described in the documentation to take an expiry period expressed in the number of days: gc.rerereResolved:: Records of conflicted merge you resolved earlier are kept for this many days when 'git rerere gc' is run. The default is 60 days. gc.rerereUnresolved:: Records of conflicted merge you have not resolved are kept for this many days when 'git rerere gc' is run. The default is 15 days. There is no strong reason not to allow a more general "approxidate" expiry specification, e.g. "5.days.ago", or "never". Rename the config_get_expiry() helper introduced in the previous step to git_config_get_expiry_in_days() and move it to a more generic place, config.c, and use date.c::parse_expiry_date() to do so. Give it an ability to allow the caller to tell among three cases (i.e. there is no "gc.rerereResolved" config, there is and it is correctly parsed into the *expiry variable, and there was an error in parsing the given value). The current caller can work correctly without using the return value, though. In the future, we may find other variables that only allow an integer that specifies "this many days" or other unit of time, and when it happens we may need to drop "_days" suffix from the name of the function and instead pass the "scale" value as another parameter. But this will do for now. Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Martin Ågren	8957661378	treewide: deprecate git_config_maybe_bool, use git_parse_maybe_bool The only difference between these is that the former takes an argument `name` which it ignores completely. Still, the callers are quite careful to provide reasonable values for it. Once in-flight topics have landed, we should be able to remove git_config_maybe_bool. In the meantime, document it as deprecated in the technical documentation. While at it, document git_parse_maybe_bool. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Martin Ågren	4666741823	config: make git_{config,parse}_maybe_bool equivalent Both of these act on a string `value` which they parse as a boolean. The "parse"-variant was introduced as a replacement for the "config"-variant which for historical reasons takes an unused argument `name`. That it was intended as a replacement is not obvious from commit `9a549d43` ("config.c: rename git_config_maybe_bool_text and export it as git_parse_maybe_bool", 2015-08-19), but that is what the background on the mailing list suggests [1]. However, these two functions do not parse `value` in exactly the same way. In particular, git_config_maybe_bool accepts integers (0 for false, non-0 for true). This means there are two slightly different definitions of "maybe_bool" in the code-base, and that every time a call to git_config_maybe_bool is changed to use git_parse_maybe_bool, it risks breaking someone's workflow. Move the implementation of "config" into "parse" and make the latter a trivial wrapper. This also fixes the only user of git_parse_maybe_bool, `git push --signed=..`. [1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/ Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Martin Ågren	9be04d64c9	config: introduce git_parse_maybe_bool_text Commit `9a549d43` ("config.c: rename git_config_maybe_bool_text and export it as git_parse_maybe_bool", 2015-08-19) intended git_parse_maybe_bool to be a replacement for git_config_maybe_bool, which could then be retired. That is not obvious from the commit message, but that is what the background on the mailing list suggests [1]. However, git_{config,parse}_maybe_bool do not handle all input the same. Before the rename, that was by design and there is a caller in config.c which requires git_parse_maybe_bool to behave exactly as it does. Prepare for the next patch by renaming git_parse_maybe_bool to ..._text and reimplementing the first one as a simple call to the second one. Let the existing users in config.c use ..._text, since it does what they need. [1] https://public-inbox.org/git/xmqq7fotd71o.fsf@gitster.dls.corp.google.com/ Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Brandon Williams	b22e51cb26	config: add config_from_gitmodules Add 'config_from_gitmodules()' function which can be used by 'fetch' and 'update_clone' in order to maintain backwards compatibility with configuration being stored in .gitmodules' since a future patch will remove reading these values in the submodule-config. This function should not be used anywhere other than in 'fetch' and 'update_clone'. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
brian m. carlson	cd73de4714	submodule: convert submodule config lookup to use object_id Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	136c8c8b8f	color: check color.ui in git_default_config() Back in prehistoric times, our decision on whether or not to show color by default relied on using a config callback that either did or didn't load color config like color.diff. When we introduced color.ui, we put it in the same boat: commands had to manually respect it by using git_color_config() or its git_color_default_config() convenience wrapper. But in `4c7f1819b` (make color.ui default to 'auto', 2013-06-10), that changed. Since then, we default color.ui to auto in all programs, meaning that even plumbing commands like "git diff-tree --pretty" might colorize the output. Nobody seems to have complained in the intervening years, presumably because the "is stdout a tty" check does a good job of catching the right cases. But that leaves an interesting curiosity: color.ui defaults to auto even in plumbing, but you can't actually _disable_ the color via config. So if you really hate color and set "color.ui" to false, diff-tree will still show color (but porcelain like git-diff won't). Nobody noticed that either, probably because very few people disable color. One could argue that the plumbing should _always_ disable color unless an explicit --color option is given on the command line. But in practice, this creates a lot of complications for scripts which do want plumbing to show user-visible output. They can't just pass "--color" blindly; they need to check the user's config and decide what to send. Given that nobody has complained about the current behavior, let's assume it's a good path, and follow it to its conclusion: supporting color.ui everywhere. Note that you can create havoc by setting color.ui=always in your config, but that's more or less already the case. We could disallow it entirely, but it is handy for one-offs like: git -c color.ui=always foo >not-a-tty when "foo" does not take a --color option itself. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Stefan Beller	77bdc09786	config.c: drop hashmap_cmp_fn cast Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Stefan Beller	7663cdc86c	hashmap.h: compare function has access to a data field When using the hashmap a common need is to have access to caller provided data in the compare function. A couple of times we abuse the keydata field to pass in the data needed. This happens for example in patch-ids.c. This patch changes the function signature of the compare function to have one more void pointer available. The pointer given for each invocation of the compare function must be defined in the init function of the hashmap and is just passed through. Documentation of this new feature is deferred to a later patch. This is a rather mechanical conversion, just adding the new pass-through parameter. However while at it improve the naming of the fields of all compare functions used by hashmaps by ensuring unused parameters are prefixed with 'unused_' and naming the parameters what they are (instead of 'unused' make it 'unused_keydata'). Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Ævar Arnfjörð Bjarmason	55d3426929	wildmatch: remove unused wildopts parameter Remove the unused wildopts placeholder struct from being passed to all wildmatch() invocations, or rather remove all the boilerplate NULL parameters. This parameter was added back in commit `9b3497cab9` ("wildmatch: rename constants and update prototype", 2013-01-01) as a placeholder for future use. Over 4 years later nothing has made use of it, let's just remove it. It can be added in the future if we find some reason to start using such a parameter. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Brandon Williams	3b256228a6	config: read config from a repository object Teach the config machinery to read config information from a repository object. This involves storing a 'struct config_set' inside the repository object and adding a number of functions (repo_config) to be able to query a repository's config. The current config API enables lazy-loading of the config. This means that when 'git_config_get_int()' is called, if the_config_set hasn't been populated yet, then it will be populated and properly initialized by reading the necessary config files (system wide .gitconfig, user's home .gitconfig, and the repository's config). To maintain this paradigm, the new API to read from a repository object's config will also perform this lazy-initialization. Since both APIs (git_config_get and repo_config_get) have the same semantics we can migrate the default config to be stored within 'the_repository' and just have the 'git_config_get' family of functions redirect to the 'repo_config_get*' functions. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Sahil Dua	52d59cc645	branch: add a --copy (-c) option to go with --move (-m) Add the ability to --copy a branch and its reflog and configuration, this uses the same underlying machinery as the --move (-m) option except the reflog and configuration is copied instead of being moved. This is useful for e.g. copying a topic branch to a new version, e.g. work to work-2 after submitting the work topic to the list, while preserving all the tracking info and other configuration that goes with the branch, and unlike --move keeping the other already-submitted branch around for reference. Like --move, when the source branch is the currently checked out branch the HEAD is moved to the destination branch. In the case of --move we don't really have a choice (other than remaining on a detached HEAD) and in order to keep the functionality consistent, we are doing it in similar way for --copy too. The most common usage of this feature is expected to be moving to a new topic branch which is a copy of the current one, in that case moving to the target branch is what the user wants, and doesn't unexpectedly behave differently than --move would. One outstanding caveat of this implementation is that: git checkout maint && git checkout master && git branch -c topic && git checkout - Will check out 'maint' instead of 'master'. This is because the @{-N} feature (or its -1 shorthand "-") relies on HEAD reflogs created by the checkout command, so in this case we'll checkout maint instead of master, as the user might expect. What to do about that is left to a future change. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Sahil Dua <sahildua2305@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Sahil Dua	5463caab15	config: create a function to format section headers Factor out the logic which creates section headers in the config file, e.g. the 'branch.foo' key will be turned into '[branch "foo"]'. This introduces no function changes, but is needed for a later change which adds support for copying branch sections in the config file. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Sahil Dua <sahildua2305@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago

1 2 3 4 5 ...

593 Commits (3ae5fa0768f7f9781b40b1d40cb2f9f4c753bad4)