* [PATCH] sha1_name: use strlcpy() to copy strings
@ 2015-02-21 19:55 René Scharfe
2015-02-22 20:00 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: René Scharfe @ 2015-02-21 19:55 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano
Use strlcpy() instead of calling strncpy() and then setting the last
byte of the target buffer to NUL explicitly. This shortens and
simplifies the code a bit.
Signed-of-by: Rene Scharfe <l.s.r@web.de>
---
sha1_name.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index cf2a83b..95f9f8f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1391,9 +1391,7 @@ static int get_sha1_with_context_1(const char *name,
namelen = strlen(cp);
}
- strncpy(oc->path, cp,
- sizeof(oc->path));
- oc->path[sizeof(oc->path)-1] = '\0';
+ strlcpy(oc->path, cp, sizeof(oc->path));
if (!active_cache)
read_cache();
@@ -1443,9 +1441,7 @@ static int get_sha1_with_context_1(const char *name,
name, len);
}
hashcpy(oc->tree, tree_sha1);
- strncpy(oc->path, filename,
- sizeof(oc->path));
- oc->path[sizeof(oc->path)-1] = '\0';
+ strlcpy(oc->path, filename, sizeof(oc->path));
free(new_filename);
return ret;
--
2.3.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sha1_name: use strlcpy() to copy strings
2015-02-21 19:55 [PATCH] sha1_name: use strlcpy() to copy strings René Scharfe
@ 2015-02-22 20:00 ` Junio C Hamano
2015-02-22 22:33 ` René Scharfe
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2015-02-22 20:00 UTC (permalink / raw)
To: René Scharfe; +Cc: Git Mailing List
René Scharfe <l.s.r@web.de> writes:
> Use strlcpy() instead of calling strncpy() and then setting the last
> byte of the target buffer to NUL explicitly. This shortens and
> simplifies the code a bit.
Thanks. It makes me wonder if the longer term direction should be
not to use a bound buffer for oc->path, though.
>
> Signed-of-by: Rene Scharfe <l.s.r@web.de>
> ---
> sha1_name.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index cf2a83b..95f9f8f 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1391,9 +1391,7 @@ static int get_sha1_with_context_1(const char *name,
> namelen = strlen(cp);
> }
>
> - strncpy(oc->path, cp,
> - sizeof(oc->path));
> - oc->path[sizeof(oc->path)-1] = '\0';
> + strlcpy(oc->path, cp, sizeof(oc->path));
>
> if (!active_cache)
> read_cache();
> @@ -1443,9 +1441,7 @@ static int get_sha1_with_context_1(const char *name,
> name, len);
> }
> hashcpy(oc->tree, tree_sha1);
> - strncpy(oc->path, filename,
> - sizeof(oc->path));
> - oc->path[sizeof(oc->path)-1] = '\0';
> + strlcpy(oc->path, filename, sizeof(oc->path));
>
> free(new_filename);
> return ret;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sha1_name: use strlcpy() to copy strings
2015-02-22 20:00 ` Junio C Hamano
@ 2015-02-22 22:33 ` René Scharfe
2015-02-23 18:36 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: René Scharfe @ 2015-02-22 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Am 22.02.2015 um 21:00 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> Use strlcpy() instead of calling strncpy() and then setting the last
>> byte of the target buffer to NUL explicitly. This shortens and
>> simplifies the code a bit.
>
> Thanks. It makes me wonder if the longer term direction should be
> not to use a bound buffer for oc->path, though.
That's a good idea in general, but a bit more involved since we'd need
to introduce a cleanup function that releases the memory allocated by
the new version of get_sha1_with_context() first and call it from the
appropriate places.
Would that be a good micro-project for GSoC or is it too simple?
René
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sha1_name: use strlcpy() to copy strings
2015-02-22 22:33 ` René Scharfe
@ 2015-02-23 18:36 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2015-02-23 18:36 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git Mailing List
On Sun, Feb 22, 2015 at 11:33:16PM +0100, René Scharfe wrote:
> Am 22.02.2015 um 21:00 schrieb Junio C Hamano:
> >René Scharfe <l.s.r@web.de> writes:
> >
> >>Use strlcpy() instead of calling strncpy() and then setting the last
> >>byte of the target buffer to NUL explicitly. This shortens and
> >>simplifies the code a bit.
> >
> >Thanks. It makes me wonder if the longer term direction should be
> >not to use a bound buffer for oc->path, though.
>
> That's a good idea in general, but a bit more involved since we'd need to
> introduce a cleanup function that releases the memory allocated by the new
> version of get_sha1_with_context() first and call it from the appropriate
> places.
>
> Would that be a good micro-project for GSoC or is it too simple?
Yeah, avoiding resource ownership questions was one of the reasons I
went with the static buffer in the first place. But I would love to see
it go away. Not only does it potentially truncate paths, but I recall
there was some complication with the size of "struct object_context" (I
couldn't find the details in a cursory search, but basically it was not
reasonable to have a big array of them).
Could we perhaps make this more like sha1_object_info_extended, where
the caller "asks" for fields by filling in pointers, and the
object_context itself can be discarded without leaking resources?
Like:
struct strbuf path = STRBUF_INIT;
struct object_context oc = OBJECT_CONTEXT_INIT;
oc.path = &path;
get_sha1_with_context(sha1, &oc);
... use path directly ...
strbuf_release(&path);
Then callers who do not care about the path do not have to even know the
feature exists (and it opens us up to adding new string-like context
fields in the future if we need to).
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-23 18:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-21 19:55 [PATCH] sha1_name: use strlcpy() to copy strings René Scharfe
2015-02-22 20:00 ` Junio C Hamano
2015-02-22 22:33 ` René Scharfe
2015-02-23 18:36 ` Jeff King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.