<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/git.git/streaming.c, branch rs/code-cleaning</title>
<subtitle>github.com: git/git.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/'/>
<entry>
<title>open_istream(): do not dereference NULL in the error case</title>
<updated>2014-02-19T00:00:53+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2014-02-19T00:00:53+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=78368f2c1ad342719ccf1e719bd5126ca6c7b68b'/>
<id>78368f2c1ad342719ccf1e719bd5126ca6c7b68b</id>
<content type='text'>
When stream-filter cannot be attached, it is expected to return NULL,
and we should close the stream we opened and signal an error by
returning NULL ourselves from this function.

However, we attempted to dereference that NULL pointer between the
point we detected the error and returned from the function.

Brought-to-attention-by: John Keeping &lt;john@keeping.me.uk&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When stream-filter cannot be attached, it is expected to return NULL,
and we should close the stream we opened and signal an error by
returning NULL ourselves from this function.

However, we attempted to dereference that NULL pointer between the
point we detected the error and returned from the function.

Brought-to-attention-by: John Keeping &lt;john@keeping.me.uk&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'ef/mingw-write'</title>
<updated>2014-01-27T18:44:59+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2014-01-27T18:44:59+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=7b4e2b7e6aba677fcefffde79d0d3a53ae623b4f'/>
<id>7b4e2b7e6aba677fcefffde79d0d3a53ae623b4f</id>
<content type='text'>
* ef/mingw-write:
  mingw: remove mingw_write
  prefer xwrite instead of write
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* ef/mingw-write:
  mingw: remove mingw_write
  prefer xwrite instead of write
</pre>
</div>
</content>
</entry>
<entry>
<title>prefer xwrite instead of write</title>
<updated>2014-01-17T20:09:26+00:00</updated>
<author>
<name>Erik Faye-Lund</name>
<email>kusmabite@gmail.com</email>
</author>
<published>2014-01-17T14:17:09+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=7edc02f4deedb3a11abeb328dc2596b2886c2f47'/>
<id>7edc02f4deedb3a11abeb328dc2596b2886c2f47</id>
<content type='text'>
Our xwrite wrapper already deals with a few potential hazards, and
are as such more robust. Prefer it instead of write to get the
robustness benefits everywhere.

Signed-off-by: Erik Faye-Lund &lt;kusmabite@gmail.com&gt;
Reviewed-and-improved-by: Jonathan Nieder &lt;jrnieder@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Our xwrite wrapper already deals with a few potential hazards, and
are as such more robust. Prefer it instead of write to get the
robustness benefits everywhere.

Signed-off-by: Erik Faye-Lund &lt;kusmabite@gmail.com&gt;
Reviewed-and-improved-by: Jonathan Nieder &lt;jrnieder@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>sha1_object_info_extended(): add an "unsigned flags" parameter</title>
<updated>2013-12-12T19:53:48+00:00</updated>
<author>
<name>Christian Couder</name>
<email>chriscool@tuxfamily.org</email>
</author>
<published>2013-12-11T07:46:07+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=de7b5d6218e4b928c5a395e34e5e1de40fae2a0d'/>
<id>de7b5d6218e4b928c5a395e34e5e1de40fae2a0d</id>
<content type='text'>
This parameter is not used yet, but it will be used to tell
sha1_object_info_extended() if it should perform object
replacement or not.

Signed-off-by: Christian Couder &lt;chriscool@tuxfamily.org&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This parameter is not used yet, but it will be used to tell
sha1_object_info_extended() if it should perform object
replacement or not.

Signed-off-by: Christian Couder &lt;chriscool@tuxfamily.org&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'jk/cat-file-batch-optim'</title>
<updated>2013-07-25T02:21:21+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2013-07-25T02:21:21+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=356df9bd8df58eb759fedaee8a8d1a7dc0872f8f'/>
<id>356df9bd8df58eb759fedaee8a8d1a7dc0872f8f</id>
<content type='text'>
If somebody wants to only know on-disk footprint of an object
without having to know its type or payload size, we can bypass a
lot of code to cheaply learn it.

* jk/cat-file-batch-optim:
  Fix some sparse warnings
  sha1_object_info_extended: pass object_info to helpers
  sha1_object_info_extended: make type calculation optional
  packed_object_info: make type lookup optional
  packed_object_info: hoist delta type resolution to helper
  sha1_loose_object_info: make type lookup optional
  sha1_object_info_extended: rename "status" to "type"
  cat-file: disable object/refname ambiguity check for batch mode
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If somebody wants to only know on-disk footprint of an object
without having to know its type or payload size, we can bypass a
lot of code to cheaply learn it.

* jk/cat-file-batch-optim:
  Fix some sparse warnings
  sha1_object_info_extended: pass object_info to helpers
  sha1_object_info_extended: make type calculation optional
  packed_object_info: make type lookup optional
  packed_object_info: hoist delta type resolution to helper
  sha1_loose_object_info: make type lookup optional
  sha1_object_info_extended: rename "status" to "type"
  cat-file: disable object/refname ambiguity check for batch mode
</pre>
</div>
</content>
</entry>
<entry>
<title>open_istream: remove unneeded check for null pointer</title>
<updated>2013-07-23T18:35:18+00:00</updated>
<author>
<name>Stefan Beller</name>
<email>stefanbeller@googlemail.com</email>
</author>
<published>2013-07-23T13:16:04+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=6a907786af835ac15962be53f1492f23e044f479'/>
<id>6a907786af835ac15962be53f1492f23e044f479</id>
<content type='text'>
'st' is allocated via xmalloc a few lines before and passed to
the stream opening functions.
The xmalloc function is written in a way that either 'st' is allocated
valid memory or xmalloc already dies.
The function calls to open_istream_* do not change 'st', as the pointer is
passed by reference and not a pointer of a pointer.

Hence 'st' cannot be null at that part of the code.

Signed-off-by: Stefan Beller &lt;stefanbeller@googlemail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
'st' is allocated via xmalloc a few lines before and passed to
the stream opening functions.
The xmalloc function is written in a way that either 'st' is allocated
valid memory or xmalloc already dies.
The function calls to open_istream_* do not change 'st', as the pointer is
passed by reference and not a pointer of a pointer.

Hence 'st' cannot be null at that part of the code.

Signed-off-by: Stefan Beller &lt;stefanbeller@googlemail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Fix some sparse warnings</title>
<updated>2013-07-18T23:43:47+00:00</updated>
<author>
<name>Ramsay Jones</name>
<email>ramsay@ramsay1.demon.co.uk</email>
</author>
<published>2013-07-18T20:25:50+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=d099b7173dabdeeb1f339151ac2169b3a91bf631'/>
<id>d099b7173dabdeeb1f339151ac2169b3a91bf631</id>
<content type='text'>
Sparse issues some "Using plain integer as NULL pointer" warnings.
Each warning relates to the use of an '{0}' initialiser expression
in the declaration of an 'struct object_info'. The first field of
this structure has pointer type. Thus, in order to suppress these
warnings, we replace the initialiser expression with '{NULL}'.

Signed-off-by: Ramsay Jones &lt;ramsay@ramsay1.demon.co.uk&gt;
Acked-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Sparse issues some "Using plain integer as NULL pointer" warnings.
Each warning relates to the use of an '{0}' initialiser expression
in the declaration of an 'struct object_info'. The first field of
this structure has pointer type. Thus, in order to suppress these
warnings, we replace the initialiser expression with '{NULL}'.

Signed-off-by: Ramsay Jones &lt;ramsay@ramsay1.demon.co.uk&gt;
Acked-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>sha1_object_info_extended: make type calculation optional</title>
<updated>2013-07-12T17:16:36+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2013-07-12T06:34:57+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=5b0864070e1e64683e49464c77a72f3c528c8f71'/>
<id>5b0864070e1e64683e49464c77a72f3c528c8f71</id>
<content type='text'>
Each caller of sha1_object_info_extended sets up an
object_info struct to tell the function which elements of
the object it wants to get. Until now, getting the type of
the object has always been required (and it is returned via
the return type rather than a pointer in object_info).

This can involve actually opening a loose object file to
determine its type, or following delta chains to determine a
packed file's base type. These effects produce a measurable
slow-down when doing a "cat-file --batch-check" that does
not include %(objecttype).

This patch adds a "typep" query to struct object_info, so
that it can be optionally queried just like size and
disk_size. As a result, the return type of the function is
no longer the object type, but rather 0/-1 for success/error.

As there are only three callers total, we just fix up each
caller rather than keep a compatibility wrapper:

  1. The simpler sha1_object_info wrapper continues to
     always ask for and return the type field.

  2. The istream_source function wants to know the type, and
     so always asks for it.

  3. The cat-file batch code asks for the type only when
     %(objecttype) is part of the format string.

On linux.git, the best-of-five for running:

  $ git rev-list --objects --all &gt;objects
  $ time git cat-file --batch-check='%(objectsize:disk)'

on a fully packed repository goes from:

  real    0m8.680s
  user    0m8.160s
  sys     0m0.512s

to:

  real    0m7.205s
  user    0m6.580s
  sys     0m0.608s

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Each caller of sha1_object_info_extended sets up an
object_info struct to tell the function which elements of
the object it wants to get. Until now, getting the type of
the object has always been required (and it is returned via
the return type rather than a pointer in object_info).

This can involve actually opening a loose object file to
determine its type, or following delta chains to determine a
packed file's base type. These effects produce a measurable
slow-down when doing a "cat-file --batch-check" that does
not include %(objecttype).

This patch adds a "typep" query to struct object_info, so
that it can be optionally queried just like size and
disk_size. As a result, the return type of the function is
no longer the object type, but rather 0/-1 for success/error.

As there are only three callers total, we just fix up each
caller rather than keep a compatibility wrapper:

  1. The simpler sha1_object_info wrapper continues to
     always ask for and return the type field.

  2. The istream_source function wants to know the type, and
     so always asks for it.

  3. The cat-file batch code asks for the type only when
     %(objecttype) is part of the format string.

On linux.git, the best-of-five for running:

  $ git rev-list --objects --all &gt;objects
  $ time git cat-file --batch-check='%(objectsize:disk)'

on a fully packed repository goes from:

  real    0m8.680s
  user    0m8.160s
  sys     0m0.512s

to:

  real    0m7.205s
  user    0m6.580s
  sys     0m0.608s

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>zero-initialize object_info structs</title>
<updated>2013-07-07T17:50:13+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2013-07-07T10:03:29+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=7c07385d902f6d8c177d533dc2faa36ef4a52a66'/>
<id>7c07385d902f6d8c177d533dc2faa36ef4a52a66</id>
<content type='text'>
The sha1_object_info_extended function expects the caller to
provide a "struct object_info" which contains pointers to
"query" items that will be filled in. The purpose of
providing pointers rather than storing the response directly
in the struct is so that callers can choose not to incur the
expense in finding particular fields that they do not care
about.

Right now the only query item is "sizep", and all callers
set it explicitly to choose whether or not to query it; they
can then leave the rest of the struct uninitialized.

However, as we add new query items, each caller will have to
be updated to explicitly turn off the new ones (by setting
them to NULL).  Instead, let's teach each caller to
zero-initialize the struct, so that they do not have to
learn about each new query item added.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The sha1_object_info_extended function expects the caller to
provide a "struct object_info" which contains pointers to
"query" items that will be filled in. The purpose of
providing pointers rather than storing the response directly
in the struct is so that callers can choose not to incur the
expense in finding particular fields that they do not care
about.

Right now the only query item is "sizep", and all callers
set it explicitly to choose whether or not to query it; they
can then leave the rest of the struct uninitialized.

However, as we add new query items, each caller will have to
be updated to explicitly turn off the new ones (by setting
them to NULL).  Instead, let's teach each caller to
zero-initialize the struct, so that they do not have to
learn about each new query item added.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>avoid infinite loop in read_istream_loose</title>
<updated>2013-03-27T20:47:02+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2013-03-25T20:21:14+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=692f0bc7ae0cb818bf3c757d509773d6f2e48c68'/>
<id>692f0bc7ae0cb818bf3c757d509773d6f2e48c68</id>
<content type='text'>
The read_istream_loose function loops on inflating a chunk of data
from an mmap'd loose object. We end the loop when we run out
of space in our output buffer, or if we see a zlib error.

We need to treat Z_BUF_ERROR specially, though, as it is not
fatal; it is just zlib's way of telling us that we need to
either feed it more input or give it more output space. It
is perfectly normal for us to hit this when we are at the
end of our buffer.

However, we may also get Z_BUF_ERROR because we have run out
of input. In a well-formed object, this should not happen,
because we have fed the whole mmap'd contents to zlib. But
if the object is truncated or corrupt, we will loop forever,
never giving zlib any more data, but continuing to ask it to
inflate.

We can fix this by considering it an error when zlib returns
Z_BUF_ERROR but we still have output space left (which means
it must want more input, which we know is a truncation
error). It would not be sufficient to just check whether
zlib had consumed all the input at the start of the loop, as
it might still want to generate output from what is in its
internal state.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The read_istream_loose function loops on inflating a chunk of data
from an mmap'd loose object. We end the loop when we run out
of space in our output buffer, or if we see a zlib error.

We need to treat Z_BUF_ERROR specially, though, as it is not
fatal; it is just zlib's way of telling us that we need to
either feed it more input or give it more output space. It
is perfectly normal for us to hit this when we are at the
end of our buffer.

However, we may also get Z_BUF_ERROR because we have run out
of input. In a well-formed object, this should not happen,
because we have fed the whole mmap'd contents to zlib. But
if the object is truncated or corrupt, we will loop forever,
never giving zlib any more data, but continuing to ask it to
inflate.

We can fix this by considering it an error when zlib returns
Z_BUF_ERROR but we still have output space left (which means
it must want more input, which we know is a truncation
error). It would not be sufficient to just check whether
zlib had consumed all the input at the start of the loop, as
it might still want to generate output from what is in its
internal state.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
