Re: [PATCH 22/67] entry.c: convert strcpy to xsnprintf

On Tue, Sep 15, 2015 at 12:01 PM, Ramsay Jones
[off-list ref] wrote:

On 15/09/15 16:40, Jeff King wrote:

quoted

This particular conversion is non-obvious, because nobody
has passed our function the length of the destination
buffer. However, the interface to checkout_entry specifies
that the buffer must be at least TEMPORARY_FILENAME_LENGTH
bytes long, so we can check that (meaning the existing code
was not buggy, but merely worrisome to somebody reading it).

Signed-off-by: Jeff King <redacted>
---
 entry.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/entry.c b/entry.c
index 1eda8e9..582c400 100644
--- a/entry.c
+++ b/entry.c

@@ -96,8 +96,8 @@ static int open_output_fd(char *path, const struct cache_entry *ce, int to_tempf
 {
      int symlink = (ce->ce_mode & S_IFMT) != S_IFREG;
      if (to_tempfile) {
-             strcpy(path, symlink
-                    ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
+             xsnprintf(path, TEMPORARY_FILENAME_LENGTH, "%s",
+                       symlink ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
              return mkstemp(path);
      } else {
              return create_file(path, !symlink ? ce->ce_mode : 0666);

Hmm, I was going to suggest strlcpy() again. However, if you expect an overflow to
occur, then xsnprintf() will at least bring it to your attention!  Checking for overflow
with strlcpy() is not rocket science either, and I guess we could add xstrlcpy() ... :-D

I mean having a default action "if overflow, then die" suits most of
the cases. It should be
a deliberate decision to not die in case of an overflow. (Why did you allocate
not enough memory in the first place?)

dunno.

ATB,
Ramsay Jones




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