From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org, "Erik Faye-Lund" <kusmabite@gmail•com>,
"Johannes Sixt" <j6t@kdbg•org>,
"Antoine Pelisse" <apelisse@gmail•com>,
"Torsten Bögershausen" <tboegi@web•de>,
"Wataru Noguchi" <wnoguchi.0727@gmail•com>,
"Johannes Schindelin" <Johannes.Schindelin@gmx•de>,
"René Scharfe" <l.s.r@web•de>,
msysGit <msysgit@googlegroups•com>
Subject: Re: [PATCH 2/2] entry.c: convert write_entry to use strbuf
Date: Wed, 23 Oct 2013 10:52:42 -0700 [thread overview]
Message-ID: <xmqqeh7bri1h.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1382532907-30561-2-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Wed, 23 Oct 2013 19:55:07 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail•com> writes:
> The strcpy call in open_output_fd() implies that the output buffer
> must be at least 25 chars long.
Hmph, where does that 25 come from?
> And it's true. The only caller that
> can trigger that code is checkout-index, which has the buffer of
> PATH_MAX chars (and any systems that have PATH_MAX shorter than 25
> chars are just insane).
>
> But in order to say that, one has to walk through a dozen of
> functions. Just convert it to strbuf to avoid the constraint and
> confusion.
Wouldn't it be far clearer to document what is going on especially
around the topath parameter to checkout_entry(), than to introduce
unnecessary strbuf overhead?
At first glance, it might appear that the caller of checkout_entry()
can specify to which path the contents are written out, but in
reality topath[] is to point at the buffer to store the temporary
path generated by the lower guts of write_entry(). It is unclear in
the original code and that is worth an in-code comment.
And when describing that API requirement, we would need to say how
big a buffer the caller must allocate for topath[] in the comment.
That size does not have to be platform-dependent PATH_MAX.
Something like this?
builtin/checkout-index.c | 2 +-
cache.h | 1 +
entry.c | 8 ++++++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index b1feda7..4ed6b23 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -14,7 +14,7 @@
static int line_termination = '\n';
static int checkout_stage; /* default to checkout stage0 */
static int to_tempfile;
-static char topath[4][PATH_MAX + 1];
+static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
static struct checkout state;
diff --git a/cache.h b/cache.h
index 85b544f..3118b7f 100644
--- a/cache.h
+++ b/cache.h
@@ -975,6 +975,7 @@ struct checkout {
refresh_cache:1;
};
+#define TEMPORARY_FILENAME_LENGTH 25
extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
struct cache_def {
diff --git a/entry.c b/entry.c
index d955af5..2df4ee1 100644
--- a/entry.c
+++ b/entry.c
@@ -234,6 +234,14 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)
return lstat(path, st);
}
+/*
+ * Write the contents from ce out to the working tree.
+ *
+ * When topath[] is not NULL, instead of writing to the working tree
+ * file named by ce, a temporary file is created by this function and
+ * its name is returned in topath[], which must be able to hold at
+ * least TEMPORARY_FILENAME_LENGTH bytes long.
+ */
int checkout_entry(struct cache_entry *ce,
const struct checkout *state, char *topath)
{
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups•com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups•com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups•com.
For more options, visit https://groups.google.com/groups/opt_out.
next prev parent reply other threads:[~2013-10-23 17:52 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-28 21:17 [PATCH] mingw-multibyte: fix memory acces violation and path length limits Wataru Noguchi
2013-09-28 23:18 ` Johannes Schindelin
2013-09-29 2:56 ` Wataru Noguchi
2013-09-29 11:01 ` [msysGit] " Stefan Beller
2013-10-01 13:37 ` Wataru Noguchi
2013-09-30 17:00 ` René Scharfe
2013-09-30 21:02 ` Erik Faye-Lund
2013-10-01 13:35 ` Wataru Noguchi
2013-10-02 22:26 ` Wataru Noguchi
2013-10-03 17:25 ` Antoine Pelisse
2013-10-03 17:36 ` Erik Faye-Lund
2013-10-05 11:39 ` Wataru Noguchi
2013-10-19 10:52 ` [PATCH] Prevent buffer overflows when path is too big Antoine Pelisse
2013-10-20 5:47 ` Torsten Bögershausen
2013-10-20 6:05 ` [msysGit] " Ondřej Bílka
2013-10-20 6:27 ` Torsten Bögershausen
2013-10-20 7:39 ` [msysGit] " Ondřej Bílka
2013-10-20 10:33 ` Duy Nguyen
2013-10-20 17:57 ` Antoine Pelisse
2013-10-21 1:31 ` Duy Nguyen
2013-10-21 19:02 ` Johannes Sixt
2013-10-21 19:07 ` Erik Faye-Lund
2013-10-21 19:14 ` Jeff King
2013-10-21 19:32 ` Jeff King
2013-10-23 12:55 ` [PATCH 1/2] entry.c: convert checkout_entry to use strbuf Nguyễn Thái Ngọc Duy
2013-10-23 12:55 ` [PATCH 2/2] entry.c: convert write_entry " Nguyễn Thái Ngọc Duy
2013-10-23 17:52 ` Junio C Hamano [this message]
2013-10-24 1:23 ` Duy Nguyen
2013-10-24 19:49 ` Junio C Hamano
2013-10-24 23:47 ` Duy Nguyen
2013-10-23 12:58 ` [PATCH 1/2] entry.c: convert checkout_entry " Antoine Pelisse
2013-10-23 13:04 ` Duy Nguyen
2013-10-23 13:06 ` Antoine Pelisse
2013-10-23 17:29 ` Jeff King
2013-10-23 17:34 ` Erik Faye-Lund
2013-10-23 17:52 ` Jeff King
2013-10-23 18:09 ` Junio C Hamano
2013-10-23 18:10 ` Jeff King
2013-10-24 1:55 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2013-10-23 12:55 ` [PATCH] Prevent buffer overflows when path is too big Duy Nguyen
2013-11-26 18:39 ` [PATCH] Prevent buffer overflows when path is too long Antoine Pelisse
2013-11-26 19:50 ` Junio C Hamano
2013-11-29 12:12 ` Antoine Pelisse
2013-12-14 11:31 ` Antoine Pelisse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqeh7bri1h.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Johannes.Schindelin@gmx$(echo .)de \
--cc=apelisse@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=j6t@kdbg$(echo .)org \
--cc=kusmabite@gmail$(echo .)com \
--cc=l.s.r@web$(echo .)de \
--cc=msysgit@googlegroups$(echo .)com \
--cc=pclouds@gmail$(echo .)com \
--cc=tboegi@web$(echo .)de \
--cc=wnoguchi.0727@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox