Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[Jeff King]

On Sat, Feb 18, 2017 at 07:12:18PM +, brian m. carlson wrote:

> On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> > So for this case, something like the patch below.
> > 
> > Incidentally, there's an off-by-one in the original loop of
> > stdin_diff_commit that reads past the end of the trailing NUL for the
> > final sha1 on the line. The problem is the:
> > 
> >   pos += GIT_SHA1_HEXSZ + 1;
> > 
> > which assumes we're slurping up the trailing space. This works in
> > practice because the caller will only permit a string which had a
> > newline (which it converted into a NUL).
> > 
> > I suspect that function could be more aggressive about complaining about
> > nonsense on the line, rather than silently ignoring it.
> 
> I'd come to basically the same patch, but I did pick up a few niceties
> from your patch, like avoiding the off-by-one issue you mentioned above.
> Can I place your sign-off on the resulting change?

Absolutely. Thanks for taking a look.

-Peff

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[brian m. carlson]

On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> So for this case, something like the patch below.
> 
> Incidentally, there's an off-by-one in the original loop of
> stdin_diff_commit that reads past the end of the trailing NUL for the
> final sha1 on the line. The problem is the:
> 
>   pos += GIT_SHA1_HEXSZ + 1;
> 
> which assumes we're slurping up the trailing space. This works in
> practice because the caller will only permit a string which had a
> newline (which it converted into a NUL).
> 
> I suspect that function could be more aggressive about complaining about
> nonsense on the line, rather than silently ignoring it.

I'd come to basically the same patch, but I did pick up a few niceties
from your patch, like avoiding the off-by-one issue you mentioned above.
Can I place your sign-off on the resulting change?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[Jeff King]

On Fri, Feb 17, 2017 at 08:42:17PM -0500, Jeff King wrote:

> > I'm wondering if parse_oid_hex could be useful here as well.
> 
> I know I haven't looked at this chunk nearly as carefully as you have,
> but it seems somewhat crazy to me that these functions get the original
> "line" in the first place. Shouldn't they get line+40 from the caller
> (who in turn should be using parse_oid_hex to compute that)?
> 
> And then each function should subsequently parse left-to-right with
> a mix of isspace() and parse_oid_hex(), and probably doesn't even need
> to care about the original "len" at all (yes, you can quit early if you
> know your len isn't long enough, but that's the unusual error case
> anyway; it's not a big deal to find that out while parsing).
> 
> In general, I think this sort of left-to-right incremental pointer
> movement is safe and simple. There may be a few cases where it doesn't
> apply (i.e., where you need to look at the end of the string to know how
> to parse the beginning), but that should be relatively rare.

So for this case, something like the patch below.

Incidentally, there's an off-by-one in the original loop of
stdin_diff_commit that reads past the end of the trailing NUL for the
final sha1 on the line. The problem is the:

  pos += GIT_SHA1_HEXSZ + 1;

which assumes we're slurping up the trailing space. This works in
practice because the caller will only permit a string which had a
newline (which it converted into a NUL).

I suspect that function could be more aggressive about complaining about
nonsense on the line, rather than silently ignoring it.

 builtin/diff-tree.c | 43 ---
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 1f1573bb2..222c671f2 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -16,37 +16,33 @@ static int diff_tree_commit_sha1(const struct object_id 
*oid)
 }
 
 /* Diff one or more commits. */
-static int stdin_diff_commit(struct commit *commit, char *line, int len)
+static int stdin_diff_commit(struct commit *commit, const char *p)
 {
struct object_id oid;
-   if (isspace(line[GIT_SHA1_HEXSZ]) && 
!get_oid_hex(line+GIT_SHA1_HEXSZ+1, )) {
-   /* Graft the fake parents locally to the commit */
-   int pos = GIT_SHA1_HEXSZ + 1;
-   struct commit_list **pptr;
-
-   /* Free the real parent list */
-   free_commit_list(commit->parents);
-   commit->parents = NULL;
-   pptr = &(commit->parents);
-   while (line[pos] && !get_oid_hex(line + pos, )) {
-   struct commit *parent = lookup_commit(oid.hash);
-   if (parent) {
-   pptr = _list_insert(parent, pptr)->next;
-   }
-   pos += GIT_SHA1_HEXSZ + 1;
+   struct commit_list **pptr = NULL;
+
+   /* Graft the fake parents locally to the commit */
+   while (isspace(*p++) && !parse_oid_hex(p, , )) {
+   struct commit *parent = lookup_commit(oid.hash);
+   if (!pptr) {
+   /* Free the real parent list */
+   free_commit_list(commit->parents);
+   commit->parents = NULL;
+   pptr = &(commit->parents);
+   }
+   if (parent) {
+   pptr = _list_insert(parent, pptr)->next;
}
}
return log_tree_commit(_tree_opt, commit);
 }
 
 /* Diff two trees. */
-static int stdin_diff_trees(struct tree *tree1, char *line, int len)
+static int stdin_diff_trees(struct tree *tree1, const char *p)
 {
struct object_id oid;
struct tree *tree2;
-   const int chunksz = GIT_SHA1_HEXSZ + 1;
-   if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
-   get_sha1_hex(line + chunksz, oid.hash))
+   if (!isspace(*p++) || parse_oid_hex(p, , ) || *p)
return error("Need exactly two trees, separated by a space");
tree2 = lookup_tree(oid.hash);
if (!tree2 || parse_tree(tree2))
@@ -64,19 +60,20 @@ static int diff_tree_stdin(char *line)
int len = strlen(line);
struct object_id oid;
struct object *obj;
+   const char *rest;
 
if (!len || line[len-1] != '\n')
return -1;
line[len-1] = 0;
-   if (get_oid_hex(line, ))
+   if (parse_oid_hex(line, , ))
return -1;
obj = parse_object(oid.hash);
if (!obj)
return -1;
if (obj->type == OBJ_COMMIT)
-   return stdin_diff_commit((struct commit *)obj, line, len);
+   return stdin_diff_commit((struct commit *)obj, rest);
if (obj->type == OBJ_TREE)
-   return stdin_diff_trees((struct tree *)obj, line, len);
+   return stdin_diff_trees((struct tree *)obj, rest);

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[Jeff King]

On Sat, Feb 18, 2017 at 01:26:07AM +, brian m. carlson wrote:

> > > + struct object_id oid;
> > >   struct tree *tree2;
> > > - if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> > > + const int chunksz = GIT_SHA1_HEXSZ + 1;
> > > + if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> > > + get_sha1_hex(line + chunksz, oid.hash))
> > 
> > I'm not sure that this is an improvement. The input expected in 'line'
> > is supposed to look like: ' +  +  + <\n>'. So your
> > 'chunk' would be a  plus one 'char' of some sort. Except that the
> > caller of this function has already replaced the newline character with
> > a '\0' char (so strlen(line) would return 81), but still passes the
> > original line length! Also, note that this (and other functions in this
> > file) actually test for 'isspace(char)' rather than for a ' ' char!
> > 
> > Hmm, maybe just:
> > 
> > if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
> > get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))
> > 
> > (or, perhaps, still call isspace() in this patch ...)
> 
> Well, I think it's strictly an improvement in that we have avoided
> writing hardcoded constants[0].  I did intend it as a "hash plus one"
> chunk, which is actually quite common throughout the code.
> 
> I'm wondering if parse_oid_hex could be useful here as well.

I know I haven't looked at this chunk nearly as carefully as you have,
but it seems somewhat crazy to me that these functions get the original
"line" in the first place. Shouldn't they get line+40 from the caller
(who in turn should be using parse_oid_hex to compute that)?

And then each function should subsequently parse left-to-right with
a mix of isspace() and parse_oid_hex(), and probably doesn't even need
to care about the original "len" at all (yes, you can quit early if you
know your len isn't long enough, but that's the unusual error case
anyway; it's not a big deal to find that out while parsing).

In general, I think this sort of left-to-right incremental pointer
movement is safe and simple. There may be a few cases where it doesn't
apply (i.e., where you need to look at the end of the string to know how
to parse the beginning), but that should be relatively rare.

> [0] If we change the hash size, all of the GIT_SHA1_HEXSZ constants can
> be replaced with a variable that varies based on hash size, and the code
> still works.

I am happy to see fewer magic numbers. But I think with incremental
pointer-movements, we don't even need to use the numeric constants,
either. If one day we can parse "sha256:1234abcd..." as an oid, then the
existing code would "just work".

-Peff

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[brian m. carlson]

On Sat, Feb 18, 2017 at 01:18:11AM +, Ramsay Jones wrote:
> 
> 
> On 18/02/17 00:06, brian m. carlson wrote:
> > Convert most leaf functions to struct object_id.  Rewrite several
> > hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
> > variable where that makes sense.
> > 
> > Signed-off-by: brian m. carlson 
> > ---
> >  builtin/diff-tree.c | 38 --
> >  1 file changed, 20 insertions(+), 18 deletions(-)
> > 
> > diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> > index 8ce00480cd..1f1573bb2a 100644
> > --- a/builtin/diff-tree.c
> > +++ b/builtin/diff-tree.c
> > @@ -7,9 +7,9 @@
> >  
> >  static struct rev_info log_tree_opt;
> >  
> > -static int diff_tree_commit_sha1(const unsigned char *sha1)
> > +static int diff_tree_commit_sha1(const struct object_id *oid)
> >  {
> > -   struct commit *commit = lookup_commit_reference(sha1);
> > +   struct commit *commit = lookup_commit_reference(oid->hash);
> > if (!commit)
> > return -1;
> > return log_tree_commit(_tree_opt, commit);
> > @@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char 
> > *sha1)
> >  /* Diff one or more commits. */
> >  static int stdin_diff_commit(struct commit *commit, char *line, int len)
> >  {
> > -   unsigned char sha1[20];
> > -   if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
> > +   struct object_id oid;
> > +   if (isspace(line[GIT_SHA1_HEXSZ]) && 
> > !get_oid_hex(line+GIT_SHA1_HEXSZ+1, )) {
> > /* Graft the fake parents locally to the commit */
> > -   int pos = 41;
> > +   int pos = GIT_SHA1_HEXSZ + 1;
> > struct commit_list **pptr;
> >  
> > /* Free the real parent list */
> > free_commit_list(commit->parents);
> > commit->parents = NULL;
> > pptr = &(commit->parents);
> > -   while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
> > -   struct commit *parent = lookup_commit(sha1);
> > +   while (line[pos] && !get_oid_hex(line + pos, )) {
> > +   struct commit *parent = lookup_commit(oid.hash);
> > if (parent) {
> > pptr = _list_insert(parent, pptr)->next;
> > }
> > -   pos += 41;
> > +   pos += GIT_SHA1_HEXSZ + 1;
> > }
> > }
> > return log_tree_commit(_tree_opt, commit);
> > @@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, 
> > char *line, int len)
> >  /* Diff two trees. */
> >  static int stdin_diff_trees(struct tree *tree1, char *line, int len)
> >  {
> > -   unsigned char sha1[20];
> > +   struct object_id oid;
> > struct tree *tree2;
> > -   if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> > +   const int chunksz = GIT_SHA1_HEXSZ + 1;
> > +   if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> > +   get_sha1_hex(line + chunksz, oid.hash))
> 
> I'm not sure that this is an improvement. The input expected in 'line'
> is supposed to look like: ' +  +  + <\n>'. So your
> 'chunk' would be a  plus one 'char' of some sort. Except that the
> caller of this function has already replaced the newline character with
> a '\0' char (so strlen(line) would return 81), but still passes the
> original line length! Also, note that this (and other functions in this
> file) actually test for 'isspace(char)' rather than for a ' ' char!
> 
> Hmm, maybe just:
> 
> if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
> get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))
> 
> (or, perhaps, still call isspace() in this patch ...)

Well, I think it's strictly an improvement in that we have avoided
writing hardcoded constants[0].  I did intend it as a "hash plus one"
chunk, which is actually quite common throughout the code.

I'm wondering if parse_oid_hex could be useful here as well.

[0] If we change the hash size, all of the GIT_SHA1_HEXSZ constants can
be replaced with a variable that varies based on hash size, and the code
still works.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[Ramsay Jones]

On 18/02/17 00:06, brian m. carlson wrote:
> Convert most leaf functions to struct object_id.  Rewrite several
> hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
> variable where that makes sense.
> 
> Signed-off-by: brian m. carlson 
> ---
>  builtin/diff-tree.c | 38 --
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> index 8ce00480cd..1f1573bb2a 100644
> --- a/builtin/diff-tree.c
> +++ b/builtin/diff-tree.c
> @@ -7,9 +7,9 @@
>  
>  static struct rev_info log_tree_opt;
>  
> -static int diff_tree_commit_sha1(const unsigned char *sha1)
> +static int diff_tree_commit_sha1(const struct object_id *oid)
>  {
> - struct commit *commit = lookup_commit_reference(sha1);
> + struct commit *commit = lookup_commit_reference(oid->hash);
>   if (!commit)
>   return -1;
>   return log_tree_commit(_tree_opt, commit);
> @@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char 
> *sha1)
>  /* Diff one or more commits. */
>  static int stdin_diff_commit(struct commit *commit, char *line, int len)
>  {
> - unsigned char sha1[20];
> - if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
> + struct object_id oid;
> + if (isspace(line[GIT_SHA1_HEXSZ]) && 
> !get_oid_hex(line+GIT_SHA1_HEXSZ+1, )) {
>   /* Graft the fake parents locally to the commit */
> - int pos = 41;
> + int pos = GIT_SHA1_HEXSZ + 1;
>   struct commit_list **pptr;
>  
>   /* Free the real parent list */
>   free_commit_list(commit->parents);
>   commit->parents = NULL;
>   pptr = &(commit->parents);
> - while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
> - struct commit *parent = lookup_commit(sha1);
> + while (line[pos] && !get_oid_hex(line + pos, )) {
> + struct commit *parent = lookup_commit(oid.hash);
>   if (parent) {
>   pptr = _list_insert(parent, pptr)->next;
>   }
> - pos += 41;
> + pos += GIT_SHA1_HEXSZ + 1;
>   }
>   }
>   return log_tree_commit(_tree_opt, commit);
> @@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, char 
> *line, int len)
>  /* Diff two trees. */
>  static int stdin_diff_trees(struct tree *tree1, char *line, int len)
>  {
> - unsigned char sha1[20];
> + struct object_id oid;
>   struct tree *tree2;
> - if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> + const int chunksz = GIT_SHA1_HEXSZ + 1;
> + if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> + get_sha1_hex(line + chunksz, oid.hash))

I'm not sure that this is an improvement. The input expected in 'line'
is supposed to look like: ' +  +  + <\n>'. So your
'chunk' would be a  plus one 'char' of some sort. Except that the
caller of this function has already replaced the newline character with
a '\0' char (so strlen(line) would return 81), but still passes the
original line length! Also, note that this (and other functions in this
file) actually test for 'isspace(char)' rather than for a ' ' char!

Hmm, maybe just:

if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))

(or, perhaps, still call isspace() in this patch ...)

ATB,
Ramsay Jones

>   return error("Need exactly two trees, separated by a space");
> - tree2 = lookup_tree(sha1);
> + tree2 = lookup_tree(oid.hash);
>   if (!tree2 || parse_tree(tree2))
>   return -1;
>   printf("%s %s\n", oid_to_hex(>object.oid),
> @@ -60,15 +62,15 @@ static int stdin_diff_trees(struct tree *tree1, char 
> *line, int len)
>  static int diff_tree_stdin(char *line)
>  {
>   int len = strlen(line);
> - unsigned char sha1[20];
> + struct object_id oid;
>   struct object *obj;
>  
>   if (!len || line[len-1] != '\n')
>   return -1;
>   line[len-1] = 0;
> - if (get_sha1_hex(line, sha1))
> + if (get_oid_hex(line, ))
>   return -1;
> - obj = parse_object(sha1);
> + obj = parse_object(oid.hash);
>   if (!obj)
>   return -1;
>   if (obj->type == OBJ_COMMIT)
> @@ -76,7 +78,7 @@ static int diff_tree_stdin(char *line)
>   if (obj->type == OBJ_TREE)
>   return stdin_diff_trees((struct tree *)obj, line, len);
>   error("Object %s is a %s, not a commit or tree",
> -   sha1_to_hex(sha1), typename(obj->type));
> +   oid_to_hex(), typename(obj->type));
>   return -1;
>  }
>  
> @@ -141,7 +143,7 @@ int cmd_diff_tree(int argc, const char **argv, const char 
> *prefix)
>   break;
>   case 1:
>   tree1 = 

[PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

[brian m. carlson]

Convert most leaf functions to struct object_id.  Rewrite several
hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
variable where that makes sense.

Signed-off-by: brian m. carlson 
---
 builtin/diff-tree.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 8ce00480cd..1f1573bb2a 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -7,9 +7,9 @@
 
 static struct rev_info log_tree_opt;
 
-static int diff_tree_commit_sha1(const unsigned char *sha1)
+static int diff_tree_commit_sha1(const struct object_id *oid)
 {
-   struct commit *commit = lookup_commit_reference(sha1);
+   struct commit *commit = lookup_commit_reference(oid->hash);
if (!commit)
return -1;
return log_tree_commit(_tree_opt, commit);
@@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char *sha1)
 /* Diff one or more commits. */
 static int stdin_diff_commit(struct commit *commit, char *line, int len)
 {
-   unsigned char sha1[20];
-   if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
+   struct object_id oid;
+   if (isspace(line[GIT_SHA1_HEXSZ]) && 
!get_oid_hex(line+GIT_SHA1_HEXSZ+1, )) {
/* Graft the fake parents locally to the commit */
-   int pos = 41;
+   int pos = GIT_SHA1_HEXSZ + 1;
struct commit_list **pptr;
 
/* Free the real parent list */
free_commit_list(commit->parents);
commit->parents = NULL;
pptr = &(commit->parents);
-   while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
-   struct commit *parent = lookup_commit(sha1);
+   while (line[pos] && !get_oid_hex(line + pos, )) {
+   struct commit *parent = lookup_commit(oid.hash);
if (parent) {
pptr = _list_insert(parent, pptr)->next;
}
-   pos += 41;
+   pos += GIT_SHA1_HEXSZ + 1;
}
}
return log_tree_commit(_tree_opt, commit);
@@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, char 
*line, int len)
 /* Diff two trees. */
 static int stdin_diff_trees(struct tree *tree1, char *line, int len)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
struct tree *tree2;
-   if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
+   const int chunksz = GIT_SHA1_HEXSZ + 1;
+   if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
+   get_sha1_hex(line + chunksz, oid.hash))
return error("Need exactly two trees, separated by a space");
-   tree2 = lookup_tree(sha1);
+   tree2 = lookup_tree(oid.hash);
if (!tree2 || parse_tree(tree2))
return -1;
printf("%s %s\n", oid_to_hex(>object.oid),
@@ -60,15 +62,15 @@ static int stdin_diff_trees(struct tree *tree1, char *line, 
int len)
 static int diff_tree_stdin(char *line)
 {
int len = strlen(line);
-   unsigned char sha1[20];
+   struct object_id oid;
struct object *obj;
 
if (!len || line[len-1] != '\n')
return -1;
line[len-1] = 0;
-   if (get_sha1_hex(line, sha1))
+   if (get_oid_hex(line, ))
return -1;
-   obj = parse_object(sha1);
+   obj = parse_object(oid.hash);
if (!obj)
return -1;
if (obj->type == OBJ_COMMIT)
@@ -76,7 +78,7 @@ static int diff_tree_stdin(char *line)
if (obj->type == OBJ_TREE)
return stdin_diff_trees((struct tree *)obj, line, len);
error("Object %s is a %s, not a commit or tree",
- sha1_to_hex(sha1), typename(obj->type));
+ oid_to_hex(), typename(obj->type));
return -1;
 }
 
@@ -141,7 +143,7 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
break;
case 1:
tree1 = opt->pending.objects[0].item;
-   diff_tree_commit_sha1(tree1->oid.hash);
+   diff_tree_commit_sha1(>oid);
break;
case 2:
tree1 = opt->pending.objects[0].item;
@@ -164,9 +166,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
   DIFF_SETUP_USE_CACHE);
while (fgets(line, sizeof(line), stdin)) {
-   unsigned char sha1[20];
+   struct object_id oid;
 
-   if (get_sha1_hex(line, sha1)) {
+   if (get_oid_hex(line, )) {
fputs(line, stdout);
fflush(stdout);