Re: [PATCH 1/3] remote-testsvn: fix unitialized variable

2012-12-15 Thread Florian Achleitner
On Friday 14 December 2012 17:11:44 Jeff King wrote:

 [...]
 We can fix it by returning -1 when no note is found (so on
 a zero return, we always found a valid value).

Good fix. Parsing of the note now always fails if the note doesn't contain the 
expected string, as it should.

 
 Signed-off-by: Jeff King p...@peff.net
 ---
 I think this is the right fix, but I am not too familiar with this code,
 so I might be missing a case where a missing Revision-number should
 provide some sentinel value (like 0) instead of returning an error. In
 fact, of the two callsites, one already does such a zero-initialization.
 
  remote-testsvn.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/remote-testsvn.c b/remote-testsvn.c
 index 51fba05..5ddf11c 100644
 --- a/remote-testsvn.c
 +++ b/remote-testsvn.c
 @@ -90,10 +90,12 @@ static int parse_rev_note(const char *msg, struct
 rev_note *res) if (end == value || i  0 || i  UINT32_MAX)
   return -1;
   res-rev_nr = i;
 + return 0;
   }
   msg += len + 1;
   }
 - return 0;
 + /* didn't find it */
 + return -1;
  }
 
  static int note2mark_cb(const unsigned char *object_sha1,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] remote-testsvn: fix unitialized variable

2012-12-14 Thread Jeff King
In remote-test-svn, there is a parse_rev_note function to
parse lines of the form Revision-number from notes. If it
finds such a line and parses it, it returns 0, copying the
value into a struct rev_note. If it finds an entry that is
garbled or out of range, it returns -1 to signal an error.

However, if it does not find any Revision-number line at
all, it returns success but does not put anything into the
rev_note. So upon a successful return, the rev_note may or
may not be initialized, and the caller has no way of
knowing.

gcc does not usually catch the use of the unitialized
variable because the conditional assignment happens in a
separate function from the point of use. However, when
compiling with -O3, gcc will inline parse_rev_note and
notice the problem.

We can fix it by returning -1 when no note is found (so on
a zero return, we always found a valid value).

Signed-off-by: Jeff King p...@peff.net
---
I think this is the right fix, but I am not too familiar with this code,
so I might be missing a case where a missing Revision-number should
provide some sentinel value (like 0) instead of returning an error. In
fact, of the two callsites, one already does such a zero-initialization.

 remote-testsvn.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/remote-testsvn.c b/remote-testsvn.c
index 51fba05..5ddf11c 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -90,10 +90,12 @@ static int parse_rev_note(const char *msg, struct rev_note 
*res)
if (end == value || i  0 || i  UINT32_MAX)
return -1;
res-rev_nr = i;
+   return 0;
}
msg += len + 1;
}
-   return 0;
+   /* didn't find it */
+   return -1;
 }
 
 static int note2mark_cb(const unsigned char *object_sha1,
-- 
1.8.0.2.4.g59402aa

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html