<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/libgit2.git/src/config_file.c, branch peff/binary-search-do-while</title>
<subtitle>github.com: libgit2/libgit2.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/'/>
<entry>
<title>config_file: refuse modifying included variables</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T20:27:19+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=1b32908973d687e4526079eeecdb82d9cb13c6e6'/>
<id>1b32908973d687e4526079eeecdb82d9cb13c6e6</id>
<content type='text'>
Modifying variables pulled in by an included file currently succeeds,
but it doesn't actually do what one would expect, as refreshing the
configuration will cause the values to reappear. As we are currently not
really able to support this use case, we will instead just return an
error for deleting and setting variables which were included via an
include.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Modifying variables pulled in by an included file currently succeeds,
but it doesn't actually do what one would expect, as refreshing the
configuration will cause the values to reappear. As we are currently not
really able to support this use case, we will instead just return an
error for deleting and setting variables which were included via an
include.
</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: move reader into `config_read` only</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T14:41:44+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=28c2cc3de33a382a5ca6858c418ecd541ab57aeb'/>
<id>28c2cc3de33a382a5ca6858c418ecd541ab57aeb</id>
<content type='text'>
Right now, we have multiple call sites which initialize a `reader`
structure. As the structure is only actually used inside of
`config_read`, we can instead just move the reader inside of the
`config_read` function. Instead, we can just pass in the configuration
file into `config_read`, which eases code readability.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Right now, we have multiple call sites which initialize a `reader`
structure. As the structure is only actually used inside of
`config_read`, we can instead just move the reader inside of the
`config_read` function. Instead, we can just pass in the configuration
file into `config_read`, which eases code readability.
</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: refresh all files if includes were modified</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T20:45:25+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=83bcd3a1911c136b4ace33fecde889c347452718'/>
<id>83bcd3a1911c136b4ace33fecde889c347452718</id>
<content type='text'>
Currently, we only re-parse the top-level configuration file when it has
changed itself. This can cause problems when an include is changed, as
we were not updating all values correctly.

Instead of conditionally reparsing only refreshed files, the logic
becomes much clearer and easier to follow if we always re-parse the
top-level configuration file when either the file itself or one of its
included configuration files has changed on disk. This commit implements
this logic.

Note that this might impact performance in some cases, as we need to
re-read all configuration files whenever any of the included files
changed. It could increase performance to just re-parse include files
which have actually changed, but this would compromise maintainability
of the code without much gain. The only case where we will gain anything
is when we actually use includes and when only these includes are
updated, which will probably be quite an unusual scenario to actually be
worthwhile to optimize.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Currently, we only re-parse the top-level configuration file when it has
changed itself. This can cause problems when an include is changed, as
we were not updating all values correctly.

Instead of conditionally reparsing only refreshed files, the logic
becomes much clearer and easier to follow if we always re-parse the
top-level configuration file when either the file itself or one of its
included configuration files has changed on disk. This commit implements
this logic.

Note that this might impact performance in some cases, as we need to
re-read all configuration files whenever any of the included files
changed. It could increase performance to just re-parse include files
which have actually changed, but this would compromise maintainability
of the code without much gain. The only case where we will gain anything
is when we actually use includes and when only these includes are
updated, which will probably be quite an unusual scenario to actually be
worthwhile to optimize.
</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: remove unused backend field from parse data</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T12:50:40+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=56a7a264e9560708f857ccae72cb709dd332a218'/>
<id>56a7a264e9560708f857ccae72cb709dd332a218</id>
<content type='text'>
The backend passed to `config_read` is never actually used anymore, so
we can remove it from the function and the `parse_data` structure.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The backend passed to `config_read` is never actually used anymore, so
we can remove it from the function and the `parse_data` structure.
</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: pass reader directly to callbacks</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T12:43:46+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=3a7f7a6eb507cfec2353b004641bc3fdc8d1bd8c'/>
<id>3a7f7a6eb507cfec2353b004641bc3fdc8d1bd8c</id>
<content type='text'>
Previously, the callbacks passed to `config_parse` got the reader via a
pointer to a pointer. This allowed the callbacks to update the callers
`reader` variable when the array holding it has been reallocated. As the
array is no longer present, we can simply the code by making the reader
a simple pointer.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Previously, the callbacks passed to `config_parse` got the reader via a
pointer to a pointer. This allowed the callbacks to update the callers
`reader` variable when the array holding it has been reallocated. As the
array is no longer present, we can simply the code by making the reader
a simple pointer.
</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: refactor include handling</title>
<updated>2017-07-15T12:14:50+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-05-31T12:34:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=73df75d8ad6e665f098c763ff03dcbdcc1d610df'/>
<id>73df75d8ad6e665f098c763ff03dcbdcc1d610df</id>
<content type='text'>
Current code for configuration files uses the `reader` structure to
parse configuration files and store additional metadata like the file's
path and checksum. These structures are stored within an array in the
backend itself, which causes multiple problems.

First, it does not make sense to keep around the file's contents with
the backend itself. While this data is usually free'd before being added
to the backend, this brings along somewhat intricate lifecycle problems.
A better solution would be to store only the file paths as well as the
checksum of the currently parsed content only.

The second problem is that the `reader` structures are stored inside an
array. When re-parsing configuration files due to changed contents, we
may cause this array to be reallocated, requiring us to update pointers
hold by callers. Furthermore, we do not keep track of includes which
are already associated to a reader inside of this array. This causes us
to add readers multiple times to the backend, e.g. in the scenario of
refreshing configurations.

This commit fixes these shortcomings. We introduce a split between the
parsing data and the configuration file's metadata. The `reader` will
now only hold the file's contents and the parser state and the new
`config_file` structure holds the file's path and checksum. Furthermore,
the new structure is a recursive structure in that it will also hold
references to the files it directly includes. The diskfile is changed to
only store the top-level configuration file.

These changes allow us further refactorings and greatly simplify
understanding the code.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Current code for configuration files uses the `reader` structure to
parse configuration files and store additional metadata like the file's
path and checksum. These structures are stored within an array in the
backend itself, which causes multiple problems.

First, it does not make sense to keep around the file's contents with
the backend itself. While this data is usually free'd before being added
to the backend, this brings along somewhat intricate lifecycle problems.
A better solution would be to store only the file paths as well as the
checksum of the currently parsed content only.

The second problem is that the `reader` structures are stored inside an
array. When re-parsing configuration files due to changed contents, we
may cause this array to be reallocated, requiring us to update pointers
hold by callers. Furthermore, we do not keep track of includes which
are already associated to a reader inside of this array. This causes us
to add readers multiple times to the backend, e.g. in the scenario of
refreshing configurations.

This commit fixes these shortcomings. We introduce a split between the
parsing data and the configuration file's metadata. The `reader` will
now only hold the file's contents and the parser state and the new
`config_file` structure holds the file's path and checksum. Furthermore,
the new structure is a recursive structure in that it will also hold
references to the files it directly includes. The diskfile is changed to
only store the top-level configuration file.

These changes allow us further refactorings and greatly simplify
understanding the code.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: use `git_buf_init` with length</title>
<updated>2017-06-08T09:58:23+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-06-07T08:20:44+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=a693b8734941889bc9a4c31752d317658162246a'/>
<id>a693b8734941889bc9a4c31752d317658162246a</id>
<content type='text'>
The `git_buf_init` function has an optional length parameter, which will
cause the buffer to be initialized and allocated in one step. This can
be used instead of static initialization with `GIT_BUF_INIT` followed by
a `git_buf_grow`. This patch does so for two functions where it is
applicable.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The `git_buf_init` function has an optional length parameter, which will
cause the buffer to be initialized and allocated in one step. This can
be used instead of static initialization with `GIT_BUF_INIT` followed by
a `git_buf_grow`. This patch does so for two functions where it is
applicable.
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge pull request #4179 from libgit2/ethomson/expand_tilde</title>
<updated>2017-05-20T15:18:07+00:00</updated>
<author>
<name>Carlos Martín Nieto</name>
<email>carlosmn@github.com</email>
</author>
<published>2017-05-20T15:18:07+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=a1023a43027207ac7a5df7233bddfe7347bee256'/>
<id>a1023a43027207ac7a5df7233bddfe7347bee256</id>
<content type='text'>
Introduce home directory expansion function for config files, attribute files</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Introduce home directory expansion function for config files, attribute files</pre>
</div>
</content>
</entry>
<entry>
<title>config_file: handle errors other than OOM while parsing section headers</title>
<updated>2017-04-04T09:58:46+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2017-03-28T07:00:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=4467aeac421efd04bc8c814541535c02853e9418'/>
<id>4467aeac421efd04bc8c814541535c02853e9418</id>
<content type='text'>
The current code in `parse_section_header_ext` is only prepared to
properly handle out-of-memory conditions for the `git_buf` structure.
While very unlikely and probably caused by a programming error, it is
also possible to run into error conditions other than out-of-memory
previous to reaching the actual parsing loop. In these cases, we will
run into undefined behavior as the `rpos` variable is only initialized
after these triggerable errors, but we use it in the cleanup-routine.

Fix the issue by unifying the function's cleanup code with an
`end_error` section, which will not use the `rpos` variable.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The current code in `parse_section_header_ext` is only prepared to
properly handle out-of-memory conditions for the `git_buf` structure.
While very unlikely and probably caused by a programming error, it is
also possible to run into error conditions other than out-of-memory
previous to reaching the actual parsing loop. In these cases, we will
run into undefined behavior as the `rpos` variable is only initialized
after these triggerable errors, but we use it in the cleanup-routine.

Fix the issue by unifying the function's cleanup code with an
`end_error` section, which will not use the `rpos` variable.
</pre>
</div>
</content>
</entry>
<entry>
<title>config, attrcache: don't fallback to dirs literally named `~`</title>
<updated>2017-03-23T12:12:39+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2017-03-23T11:59:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=29aef94830edb6231a0737cfca35233f9a95463f'/>
<id>29aef94830edb6231a0737cfca35233f9a95463f</id>
<content type='text'>
The config and attrcache file reading code would attempt to load a file
in a home directory by expanding the `~` and looking for the file, using
`git_sysdir_find_global_file`.  If the file was not found, the error
handling would look for the literal path, eg `~/filename.txt`.

Use the new `git_config_expand_global_file` instead, which allows us to
get the path to the file separately, when the path is prefixed with
`~/`, and fail with a not found error without falling back to looking
for the literal path.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The config and attrcache file reading code would attempt to load a file
in a home directory by expanding the `~` and looking for the file, using
`git_sysdir_find_global_file`.  If the file was not found, the error
handling would look for the literal path, eg `~/filename.txt`.

Use the new `git_config_expand_global_file` instead, which allows us to
get the path to the file separately, when the path is prefixed with
`~/`, and fail with a not found error without falling back to looking
for the literal path.
</pre>
</div>
</content>
</entry>
</feed>
