On Tue, Sep 15, 2015 at 12:01 PM, Ramsay Jones
<ramsay@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>
> On 15/09/15 16:40, Jeff King wrote:
>> 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 <peff@xxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html