Re: [PATCH 53/67] drop strcpy in favor of raw sha1_to_hex

[Jeff King]

On Fri, Sep 18, 2015 at 03:24:37PM -0400, Eric Sunshine wrote:

> > fprintf(stderr, "updating '%s'", ref->name);
> > if (strcmp(ref->name, ref->peer_ref->name))
> > fprintf(stderr, " using '%s'", ref->peer_ref->name);
> > -   fprintf(stderr, "\n  from %s\n  to   %s\n", old_hex, 
> > new_hex);
> > +   fprintf(stderr, "\n  from %s\n  to   %s\n",
> > +   sha1_to_hex(ref->old_sha1), 
> > sha1_to_hex(ref->new_sha1));
> 
> Would it make sense for the commit message can mention that when this
> code was written originally, it was not safe to call sha1_to_hex()
> twice like this within a single expression, but became safe as of
> dcb3450 (sha1_to_hex() usage cleanup, 2006-05-03)?

Sure. I suspected that was the case (there were several spots like this
in http-push.c), but didn't actually dig.

-Peff
--
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

Re: [PATCH 53/67] drop strcpy in favor of raw sha1_to_hex

[Eric Sunshine]

On Tue, Sep 15, 2015 at 12:06 PM, Jeff King  wrote:
> In some cases where we strcpy() the result of sha1_to_hex(),
> there's no need; the result goes directly into a printf
> statement, and we can simply pass the return value from
> sha1_to_hex() directly.
>
> Signed-off-by: Jeff King 
> ---
> diff --git a/http-push.c b/http-push.c
> index 43a9036..48f39b7 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -1856,7 +1856,6 @@ int main(int argc, char **argv)
>
> new_refs = 0;
> for (ref = remote_refs; ref; ref = ref->next) {
> -   char old_hex[60], *new_hex;
> struct argv_array commit_argv = ARGV_ARRAY_INIT;
>
> if (!ref->peer_ref)
> @@ -1911,13 +1910,12 @@ int main(int argc, char **argv)
> }
> hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
> new_refs++;
> -   strcpy(old_hex, sha1_to_hex(ref->old_sha1));
> -   new_hex = sha1_to_hex(ref->new_sha1);
>
> fprintf(stderr, "updating '%s'", ref->name);
> if (strcmp(ref->name, ref->peer_ref->name))
> fprintf(stderr, " using '%s'", ref->peer_ref->name);
> -   fprintf(stderr, "\n  from %s\n  to   %s\n", old_hex, new_hex);
> +   fprintf(stderr, "\n  from %s\n  to   %s\n",
> +   sha1_to_hex(ref->old_sha1), 
> sha1_to_hex(ref->new_sha1));

Would it make sense for the commit message can mention that when this
code was written originally, it was not safe to call sha1_to_hex()
twice like this within a single expression, but became safe as of
dcb3450 (sha1_to_hex() usage cleanup, 2006-05-03)?

> if (dry_run) {
> if (helper_status)
> printf("ok %s\n", ref->name);
--
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 53/67] drop strcpy in favor of raw sha1_to_hex

[Jeff King]

In some cases where we strcpy() the result of sha1_to_hex(),
there's no need; the result goes directly into a printf
statement, and we can simply pass the return value from
sha1_to_hex() directly.

Signed-off-by: Jeff King 
---
 http-push.c | 6 ++
 walker.c| 5 ++---
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/http-push.c b/http-push.c
index 43a9036..48f39b7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1856,7 +1856,6 @@ int main(int argc, char **argv)
 
new_refs = 0;
for (ref = remote_refs; ref; ref = ref->next) {
-   char old_hex[60], *new_hex;
struct argv_array commit_argv = ARGV_ARRAY_INIT;
 
if (!ref->peer_ref)
@@ -1911,13 +1910,12 @@ int main(int argc, char **argv)
}
hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
new_refs++;
-   strcpy(old_hex, sha1_to_hex(ref->old_sha1));
-   new_hex = sha1_to_hex(ref->new_sha1);
 
fprintf(stderr, "updating '%s'", ref->name);
if (strcmp(ref->name, ref->peer_ref->name))
fprintf(stderr, " using '%s'", ref->peer_ref->name);
-   fprintf(stderr, "\n  from %s\n  to   %s\n", old_hex, new_hex);
+   fprintf(stderr, "\n  from %s\n  to   %s\n",
+   sha1_to_hex(ref->old_sha1), sha1_to_hex(ref->new_sha1));
if (dry_run) {
if (helper_status)
printf("ok %s\n", ref->name);
diff --git a/walker.c b/walker.c
index 44a936c..cdeb63f 100644
--- a/walker.c
+++ b/walker.c
@@ -17,10 +17,9 @@ void walker_say(struct walker *walker, const char *fmt, 
const char *hex)
 
 static void report_missing(const struct object *obj)
 {
-   char missing_hex[41];
-   strcpy(missing_hex, sha1_to_hex(obj->sha1));
fprintf(stderr, "Cannot obtain needed %s %s\n",
-   obj->type ? typename(obj->type): "object", missing_hex);
+   obj->type ? typename(obj->type): "object",
+   sha1_to_hex(obj->sha1));
if (!is_null_sha1(current_commit_sha1))
fprintf(stderr, "while processing commit %s.\n",
sha1_to_hex(current_commit_sha1));
-- 
2.6.0.rc2.408.ga2926b9

--
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