- 30 Apr, 2019 1 commit
-
-
Opening a default config when ~/.gitconfig doesn't exist, locking it, and attempting to write to it causes an assertion failure. Treat non-existent global config file content as an empty string.
Ian Hattendorf committed
-
- 16 Apr, 2019 2 commits
-
-
git_array_alloc can return NULL if no memory is available, causing a segmentation fault in memset. This adds GIT_ERROR_CHECK_ALLOC similar to how other parts of the code base deal with the return value of git_array_alloc.
Tobias Nießen committed -
Patrick Steinhardt committed
-
- 22 Jan, 2019 1 commit
-
-
Move to the `git_error` name in the internal API for error-related functions.
Edward Thomson committed
-
- 05 Oct, 2018 1 commit
-
-
In case a configuration includes a key "include.path=" without any value, the generated configuration entry will have its value set to `NULL`. This is unexpected by the logic handling includes, and as soon as we try to calculate the included path we will unconditionally dereference that `NULL` pointer and thus segfault. Fix the issue by returning early in both `parse_include` and `parse_conditional_include` in case where the `file` argument is `NULL`. Add a test to avoid future regression. The issue has been found by the oss-fuzz project, issue 10810.
Patrick Steinhardt committed
-
- 28 Sep, 2018 9 commits
-
-
Right now, the config file code requires us to pass in its backend to the config entry iterator. This is required with the current code, as the config file backend will first create a read-only snapshot which is then passed to the iterator just for that purpose. So after the iterator is getting free'd, the code needs to make sure that the snapshot gets free'd, as well. By now, though, we can easily refactor the code to be more efficient and remove the reverse dependency from iterator to backend. Instead of creating a read-only snapshot (which also requires us to re-parse the complete configuration file), we can simply duplicate the config entries and pass those to the iterator. Like that, the iterator only needs to make sure to free the duplicated config entries, which is trivial to do and clears up memory ownership by a lot.
Patrick Steinhardt committed -
Instead of directly calling `git_atomic_inc` in users of the config entries store, provide a `git_config_entries_incref` function to further decouple the interfaces. Convert the refcount to a `git_refcount` structure while at it.
Patrick Steinhardt committed -
The nice thing about our `git_config_iterator` interfaces is that nobody needs to know anything about the implementation details. All that is required is to obtain the iterator via any backend and then use it by executing generic functions. We can thus completely internalize all the implementation details of how to iterate over entries into the config entries store and simply create such an iterator in our config file backend when we want to iterate its entries. This further decouples the config file backend from the config entries store.
Patrick Steinhardt committed -
The code accessing config entries in the `git_config_entries` structure is still much too intimate with implementation details, directly accessing the maps and handling indices. Provide two new functions to get config entries from the internal map structure to decouple the interfaces and use them in the config file code. The function `git_config_entries_get` will simply look up the entry by name and, in the case of a multi-value, return the last occurrence of that entry. The second function, `git_config_entries_get_unique`, will only return an entry if it is unique and not included via another configuration file. This one is required to properly implement write operations for single entries, as we refuse to write to or delete a single entry if it is not clear which one was meant.
Patrick Steinhardt committed -
The previous commit simply moved all code that is required to handle config entries to a new module without yet adjusting any of the function and structure names to help readability. We now rename things accordingly to have a common "git_config_entries" entries instead of the old "diskfile_entries" one.
Patrick Steinhardt committed -
The configuration entry store that is used for configuration files needs to keep track of all entries in two different structures: - a singly linked list is being used to be able to iterate through configuration files in the order they have been found - a string map is being used to efficiently look up configuration entries by their key This store is thus something that may be used by other, future backends as well to abstract away implementation details and iteration over the entries. Pull out the necessary functions from "config_file.c" and moves them into their own "config_entries.c" module. For now, this is simply moving over code without any renames and/or refactorings to help reviewing.
Patrick Steinhardt committed -
The implementation for config file snapshots has an unnecessary redirection from `config_snapshot` to `git_config_file__snapshot`. Inline the call to `git_config_file__snapshot` and remove it.
Patrick Steinhardt committed -
The header "config_file.h" has a list of inline-functions to access the contents of a config backend without directly messing with the struct's function pointers. While all these functions are called "git_config_file_*", they are in fact completely backend-agnostic and don't care whether it is a file or not. Rename all the function to instead be backend-agnostic versions called "git_config_backend_*" and rename the header to match.
Patrick Steinhardt committed -
The function `git_config_file_normalize_section` is never being used in any file different than "config.c", but it is implemented in "config_file.c". Move it over and make the symbol static.
Patrick Steinhardt committed
-
- 06 Sep, 2018 1 commit
-
-
In case where we add multiple configuration entries with the same key to a diskfile backend, we always need to iterate the list of this key to find the last entry due to the list being a singly-linked list. This is obviously quadratic behaviour, and this has sure enough been found by oss-fuzz by generating a configuration file with 50k lines, where most of them have the same key. While the issue will not arise with "sane" configuration files, an adversary may trigger it by providing a crafted ".gitmodules" file, which is delivered as part of the repo and also parsed by the configuration parser. The fix is trivial: store a pointer to the last entry of the list in its head. As there are only two locations now where we append to this data structure, mainting this pointer is trivial, too. We can also optimize retrieval of a single value via `config_get`, where we previously had to chase the `next` pointer to find the last entry that was added. Using our configuration file fozzur with a corpus that has a single file with 50000 "-=" lines previously took around 21s. With this optimization the same file scans in about 0.053s, which is a nearly 400-fold improvement. But in most cases with a "normal" amount of same-named keys it's not going to matter anyway.
Patrick Steinhardt committed
-
- 05 Aug, 2018 3 commits
-
-
Nelson Elhage committed
-
Nelson Elhage committed
-
Nelson Elhage committed
-
- 22 Jun, 2018 2 commits
-
-
Buffers which ran out of memory will never have any memory attached to them. As such, it is not necessary to call `git_buf_free` if the buffer is out of memory.
Patrick Steinhardt committed -
The function `git_config_parse` uses several callbacks to pass data along to the caller as it parses the file. One design shortcoming here is that strings passed to those callbacks are expected to be freed by them, which is really confusing. Fix the issue by changing memory ownership here. Instead of expecting the `on_variable` callbacks to free memory for `git_config_parse`, just do it inside of `git_config_parse`. While this obviously requires a bit more memory allocation churn due to having to copy both name and value at some places, this shouldn't be too much of a burden.
Patrick Steinhardt committed
-
- 10 Jun, 2018 1 commit
-
-
Patrick Steinhardt committed
-
- 26 Mar, 2018 11 commits
-
-
Currently, all configuration entries were only held in a string map, making iteration order mostly based on the hash of each entry's key. Now that we have extended the `diskfile_entries` structure by a list of config entries, we can effectively iterate through entries in the order they were added, though.
Patrick Steinhardt committed -
Right now, we only keep all configuration entries in a string map. This is required to efficiently access configuration entries by keys. It has the disadvantage of not being able to iterate through configuration entries in the order they were read, though. Instead, we only iterate through entries in a seemingly random order. Extend `diskfile_entries` by another list holding configuration entries. Like this, we maintain all entries in two data structures and can use the required one based on the current use case.
Patrick Steinhardt committed -
Currently, we only parse the entry map into `append_entry` to append new configuration entries to it. Instead, though, we can just pass the complete `diskfile_entries` structure into it. This allows us to easily extend `diskfile_entries` by another singly linked list of configuration entries.
Patrick Steinhardt committed -
The config file parsing code all revolves around the `refcounted_strmap` structure, which is a map of entry names to their respective keys. This naming scheme made grasping the code quite hard, warranting a rename. A good alternative is `diskfile_entries`, making clear that this really only holds all configuration entries. Furthermore, we are about to introduce a new linked list of configuration entries into the configuration file code. This list will be used to iterate over configuration entries in the order they are listed inside of the parsed configuration file. After renaming `refcounted_strmap` to `diskfile_entries`, this struct also becomes the natural target where to add that new list. Like this, data structures holding all entries are neatly contained inside of it.
Patrick Steinhardt committed -
The struct `parse_data` sounds as if it was defined and passed to us from the configuration parser, which is not true. Instead, `parse_data` is specific to the diskfile backend parsing logic. Rename it to `diskfile_parse_state` to make that clearer. This also follows existing naming patterns with the "diskfile" prefix.
Patrick Steinhardt committed -
Patrick Steinhardt committed
-
The interface for freeing config list entries is more tangled than required. Instead of calling `cvar_free` for every list entry in `free_vars`, we now just provide a function `config_entry_list_free`. This function will iterate through all list entries and free the associated configuration entry as well as the list entry itself.
Patrick Steinhardt committed -
The `cvar_t` structure is really awkward to grasp, because its name actively hinders discovery of what it actually is. As it is nothing more than a singly-linked list of configuration entries, name rename it to just that: `config_entry_list`.
Patrick Steinhardt committed -
In order to reject writes to included configuration entries, we need to keep track of whether an entry was included via another configuration file or not. This information is being stored in the `cvar` structure, which is a rather weird location, as it is only used to create a list structure of config entries. Move the include depth into the structure `git_config_entry` instead. While this fixes the layering issue, it enables users of libgit2 to access the depth, too.
Patrick Steinhardt committed -
The code appending new configuration entries to our current list first allocates the `cvar` structure and then passes it to `append_entry`. As we want to extend `append_entry` to store configuration entries in a map as well as in a list for ordered iteration, we will have to create two `cvar` structures, though. As such, the future change will become much easier when allocation of the `cvar` structure is doen in `append_entry` itself.
Patrick Steinhardt committed -
We currently provide a lot of macros for the `cvar_t` structure which are never being used. In fact, the only macro we need is to access the `next` pointer of `cvar_t`, which really does not require a macro at all. Remove all these macros and replace usage of `CVAR_LIST_NEXT(cvar)` with `cvar->next`.
Patrick Steinhardt committed
-
- 28 Feb, 2018 4 commits
-
-
Instead of treating it as a no-op, treat it as a programming error and return the same kind of error as if you called to set or delete variables on a snapshot.
Carlos Martín Nieto committed -
When we create an iterator we don't actually know that we have a live config object and we must instead only rely on the header. We fixed it to use this in a previous commit, but this makes it harder to misuse by converting to use the header object in the typecast. We also guard inside the `config_refresh` function against being given a snapshot (although callers right now do check).
Carlos Martín Nieto committed -
We use it in a few places where we might have a full object or a snapshot so move it to where we can actually access it.
Carlos Martín Nieto committed -
We pass this around and when creating a new iterator we need to read the repository pointer. Put it in a common place so we can reach it regardless of whether we got a full object or a snapshot.
Carlos Martín Nieto committed
-
- 11 Nov, 2017 2 commits
-
-
As the config parser is now cleanly separated from the config file code, we can easily refactor the code and make use of the common parser module. This removes quite a lot of duplicated functionality previously used for handling the actual parser state and replaces it with the generic interface provided by the parser context.
Patrick Steinhardt committed -
The configuration file code grew quite big and intermingles both actual configuration logic as well as the parsing logic of the configuration syntax. This makes it hard to refactor the parsing logic on its own and convert it to make use of our new parsing context module. Refactor the code and split it up into two parts. The config file code will only handle actual handling of configuration files, includes and writing new files. The newly created config parser module is then only responsible for parsing the actual contents of a configuration file, leaving everything else to callbacks provided to its provided function `git_config_parse`.
Patrick Steinhardt committed
-
- 04 Nov, 2017 1 commit
-
-
Carlos Martín Nieto committed
-
- 30 Oct, 2017 1 commit
-
-
Carlos Martín Nieto committed
-