{"thread":{"id":"36765","subject":"Git chokes on large file","startedAt":"2014-05-27T16:47:13Z","lastAt":"2014-08-16T03:08:06Z","messageCount":49,"participants":["Dale R. Worley","Duy Nguyen","Thomas Braun","Junio C Hamano","David Lang","Nguyễn Thái Ngọc Duy","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"242752","messageId":"201405271647.s4RGlDJc024596@hobgoblin.ariadne.com","threadId":"36765","inReplyTo":null,"subject":"Git chokes on large file","fromName":"Dale R. Worley","fromEmail":"worley@alum.mit.edu","sentAt":"2014-05-27T16:47:13Z","receivedAt":"2014-05-27T16:47:13Z","isPatch":false,"sender":{"key":"worley@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/19911107?v=4"},"body":"I've discovered a problem using Git.  It's not clear to me what the\n\"correct\" behavior should be, but it seems to me that Git is failing\nin an undesirable way.\n\nThe problem arises when trying to handle a very large file.  For\nexample:\n\n    $ git --version\n    git version 1.8.3.1\n    $ mkdir $$\n    $ cd $$\n    $ git init\n    Initialized empty Git repository in /common/not-replicated/worley/temp/5627/.git/\n    $ truncate --size=20G big_file\n    $ ls -l\n    total 0\n    -rw-rw-r--. 1 worley worley 21474836480 May 27 11:59 big_file\n    $ time git add big_file\n\n    real\t4m48.752s\n    user\t4m31.295s\n    sys\t0m16.747s\n    $\n\nAt this point, either 'git fsck' or 'git commit' fails:\n\n    $ git fsck --full --strict\n    notice: HEAD points to an unborn branch (master)\n    Checking object directories: 100% (256/256), done.\n    fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\n    $ git commit -m Test.\n    [master (root-commit) 3df3655] Test.\n    fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nThe central problem is that one can accidentally add a file that\nleaves the repository in a \"broken\" state, where various normal\ncommands simply don't work.  The most worrying aspect is that \"git\nfsck\" fails -- of all the commands, the one that verifies the validity\nof the repository (and diagnoses errors) should be the most robust!\n\nEven doing a 'git reset' does not put the repository in a state where\n'git fsck' will complete:\n\n    $ git reset\n    $ git fsck --full --strict\n    notice: HEAD points to an unborn branch (master)\n    Checking object directories: 100% (256/256), done.\n    fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nDale\n"},{"id":"242855","messageId":"CACsJy8BM1f1pJPzGPf--a-kUim6wyX+Mr1AfMupY3mpREY+8DA@mail.gmail.com","threadId":"36765","inReplyTo":"201405271647.s4RGlDJc024596@hobgoblin.ariadne.com","subject":"Re: Git chokes on large file","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-28T13:32:16Z","receivedAt":"2014-05-28T13:32:16Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 27, 2014 at 11:47 PM, Dale R. Worley <worley@alum.mit.edu> wrote:\n> I've discovered a problem using Git.  It's not clear to me what the\n> \"correct\" behavior should be, but it seems to me that Git is failing\n> in an undesirable way.\n>\n> The problem arises when trying to handle a very large file.  For\n> example:\n>\n>     $ git --version\n>     git version 1.8.3.1\n>     $ mkdir $$\n>     $ cd $$\n>     $ git init\n>     Initialized empty Git repository in /common/not-replicated/worley/temp/5627/.git/\n>     $ truncate --size=20G big_file\n>     $ ls -l\n>     total 0\n>     -rw-rw-r--. 1 worley worley 21474836480 May 27 11:59 big_file\n>     $ time git add big_file\n>\n>     real        4m48.752s\n>     user        4m31.295s\n>     sys 0m16.747s\n>     $\n>\n> At this point, either 'git fsck' or 'git commit' fails:\n>\n>     $ git fsck --full --strict\n>     notice: HEAD points to an unborn branch (master)\n>     Checking object directories: 100% (256/256), done.\n>     fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nBack trace for this one\n\n#3  0x000000000055cf39 in xmalloc (size=21474836481) at wrapper.c:49\n#4  0x000000000055cffd in xmallocz (size=21474836480) at wrapper.c:73\n#5  0x0000000000537858 in unpack_compressed_entry (p=0x858ac0,\nw_curs=0x7fffffffc0f8, curpos=18, size=21474836480) at\nsha1_file.c:1924\n#6  0x0000000000538364 in unpack_entry (p=0x858ac0, obj_offset=12,\nfinal_type=0x7fffffffc1e4, final_size=0x7fffffffc1d8) at\nsha1_file.c:2206\n#7  0x00000000004fb0a2 in verify_packfile (p=0x858ac0,\nw_curs=0x7fffffffc320, fn=0x43f5f2 <fsck_obj_buffer>,\nprogress=0x858a90, base_count=0) at pack-check.c:119\n#8  0x00000000004fb3f4 in verify_pack (p=0x858ac0, fn=0x43f5f2\n<fsck_obj_buffer>, progress=0x858a90, base_count=0) at\npack-check.c:177\n#9  0x00000000004401d7 in cmd_fsck (argc=0, argv=0x7fffffffd650,\nprefix=0x0) at builtin/fsck.c:677\n\nNot easy to fix. I started working on converting fsck to use\nindex-pack code for pack verification. index-pack supports large files\nwell, so in the end it might fix this (as well as speeding up fsck).\nBut that work has stalled for a long time.\n\n>\n>     $ git commit -m Test.\n>     [master (root-commit) 3df3655] Test.\n>     fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nAnd back trace\n\n#11 0x00000000004b9da0 in read_sha1_file (sha1=0x8558a0\n\"\\256/s\\324\\370\\304\\344\\212\\304I\\v\\342\\334MS\\002\\352\\214\\061\\222\",\ntype=0x7fffffffc6c4, size=0x8558d0) at cache.h:820\n#12 0x00000000004c1b98 in diff_populate_filespec (s=0x8558a0,\nsize_only=0) at diff.c:2749\n#13 0x00000000004c0110 in diff_filespec_is_binary (one=0x8558a0) at diff.c:2188\n#14 0x00000000004c0f0b in builtin_diffstat (name_a=0x858530\n\"big_file\", name_b=0x0, one=0x8584e0, two=0x8558a0,\ndiffstat=0x7fffffffc8a0, o=0x7fffffffce88, p=0x855910) at diff.c:2435\n#15 0x00000000004c2fd4 in run_diffstat (p=0x855910, o=0x7fffffffce88,\ndiffstat=0x7fffffffc8a0) at diff.c:3168\n#16 0x00000000004c603a in diff_flush_stat (p=0x855910,\no=0x7fffffffce88, diffstat=0x7fffffffc8a0) at diff.c:4081\n#17 0x00000000004c70e4 in diff_flush (options=0x7fffffffce88) at diff.c:4520\n#18 0x00000000004e5d59 in log_tree_diff_flush (opt=0x7fffffffcaf0) at\nlog-tree.c:715\n#19 0x00000000004e5e5a in log_tree_diff (opt=0x7fffffffcaf0,\ncommit=0x8585b0, log=0x7fffffffc9a0) at log-tree.c:747\n#20 0x00000000004e60b1 in log_tree_commit (opt=0x7fffffffcaf0,\ncommit=0x8585b0) at log-tree.c:810\n#21 0x000000000042c45c in print_summary (prefix=0x0,\nsha1=0x7fffffffd300 \".&Gȑ\\360\\243\\202\\351&!\\035\\312q\\374\\345\\314LL)\",\ninitial_commit=1) at builtin/commit.c:1426\n#22 0x000000000042d213 in cmd_commit (argc=0, argv=0x7fffffffd650,\nprefix=0x0) at builtin/commit.c:1750\n\nIf we could have an option in read_sha1_file to read max to <n> bytes\n(enough for binary detection purpose), it would fix this. Another\noption is declare all files larger than core.bigfilethreshold binary.\nEasier in both senses of implementation cost and looseness.\n\n> Even doing a 'git reset' does not put the repository in a state where\n> 'git fsck' will complete:\n>\n>     $ git reset\n>     $ git fsck --full --strict\n>     notice: HEAD points to an unborn branch (master)\n>     Checking object directories: 100% (256/256), done.\n>     fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nI don't know how many commands are hit by this. If you have time and\ngdb, please put a break point in die_builtin() function and send\nbacktraces for those that fail. You could speed up the process by\ncreating a smaller file and set the environment variable\nGIT_ALLOC_LIMIT (in kilobytes) to a number lower than that size. If\ngit attempts to allocate a block larger than that limit it'll die.\n-- \nDuy\n"},{"id":"242863","messageId":"5385FB56.6020009@virtuell-zuhause.de","threadId":"36765","inReplyTo":"201405271647.s4RGlDJc024596@hobgoblin.ariadne.com","subject":"Re: Git chokes on large file","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2014-05-28T15:05:58Z","receivedAt":"2014-05-28T15:05:58Z","isPatch":false,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am 27.05.2014 18:47, schrieb Dale R. Worley:\n> Even doing a 'git reset' does not put the repository in a state where\n> 'git fsck' will complete:\n\nYou have to remove the offending commit also from the reflog.\n\nThe following snipped creates an offending commit, big_file is 2GB which\nis too large for git on windows, and later removes it completely so that\ngit fsck passes again.\n\n-----------------------------------\ngit init\necho 1 > some_file\ngit add some_file\ngit commit -m \"add some_file\"\ngit add big_file\ngit commit -m \"add big_file\" # reports malloc error without the -q flag\ngit log --stat # malloc error\ngit reset HEAD^1\ngit fsck --full --strict --verbose # fails\ngit reflog expire --expire=now --all # remove all reflog entries\ngit gc --prune=now\ngit fsck --full --strict --verbose # passes\n-----------------------------------\n"},{"id":"242871","messageId":"xmqqbnuhesm5.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"CACsJy8BM1f1pJPzGPf--a-kUim6wyX+Mr1AfMupY3mpREY+8DA@mail.gmail.com","subject":"Re: Git chokes on large file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-28T17:10:58Z","receivedAt":"2014-05-28T17:10:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n>>     $ git fsck --full --strict\n>>     notice: HEAD points to an unborn branch (master)\n>>     Checking object directories: 100% (256/256), done.\n>>     fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n>\n> Back trace for this one\n> ...\n> Not easy to fix. I started working on converting fsck to use\n> index-pack code for pack verification. index-pack supports large files\n> well, so in the end it might fix this (as well as speeding up fsck).\n> But that work has stalled for a long time.\n\nYou need to have enough memory (virtual is fine if you have enough\ntime) to do fsck.  Some part of index-pack could be refactored into\na common helper function that could be called from fsck, but I think\nit would be a lot of work.\n\n>>     $ git commit -m Test.\n>>     [master (root-commit) 3df3655] Test.\n>>     fatal: Out of memory, malloc failed (tried to allocate 21474836481 bytes)\n\nI suspect that this one is only because you are letting the status\nwhich involves diff to kick in (and for that you would need to have\nenough memory).  As you suggested, it might be a good idea to take\nbigfilethreshold account when deciding if we would want to run diff.\n"},{"id":"242880","messageId":"201405281815.s4SIF5hF025886@hobgoblin.ariadne.com","threadId":"36765","inReplyTo":"CACsJy8BM1f1pJPzGPf--a-kUim6wyX+Mr1AfMupY3mpREY+8DA@mail.gmail.com","subject":"Re: Git chokes on large file","fromName":"Dale R. Worley","fromEmail":"worley@alum.mit.edu","sentAt":"2014-05-28T18:15:05Z","receivedAt":"2014-05-28T18:15:05Z","isPatch":false,"sender":{"key":"worley@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/19911107?v=4"},"body":"> From: Duy Nguyen <pclouds@gmail.com>\n\n> I don't know how many commands are hit by this. If you have time and\n> gdb, please put a break point in die_builtin() function and send\n> backtraces for those that fail. You could speed up the process by\n> creating a smaller file and set the environment variable\n> GIT_ALLOC_LIMIT (in kilobytes) to a number lower than that size. If\n> git attempts to allocate a block larger than that limit it'll die.\n\nI don't use Git enough to exercise it well.  And there are dozens of\ncommands with hundreds of options.\n\nAs someone else has noted, if I run 'git commit -q --no-status', it\ndoesn't crash.\n\nIt seems that much of Git was coded under the assumption that any file\ncould always be held entirely in RAM.  Who made that mistake?  Are\npeople so out of touch with reality?\n\nDale\n"},{"id":"242881","messageId":"201405281818.s4SIIV5n026003@hobgoblin.ariadne.com","threadId":"36765","inReplyTo":"xmqqbnuhesm5.fsf@gitster.dls.corp.google.com","subject":"Re: Git chokes on large file","fromName":"Dale R. Worley","fromEmail":"worley@alum.mit.edu","sentAt":"2014-05-28T18:18:31Z","receivedAt":"2014-05-28T18:18:31Z","isPatch":false,"sender":{"key":"worley@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/19911107?v=4"},"body":"> From: Junio C Hamano <gitster@pobox.com>\n\n> You need to have enough memory (virtual is fine if you have enough\n> time) to do fsck.  Some part of index-pack could be refactored into\n> a common helper function that could be called from fsck, but I think\n> it would be a lot of work.\n\nHow much memory is \"enough\"?  And how is it that fsck is coded so that\nthe available RAM is a limit on which repositories can be checked?\nDid someone forget that RAM may be considerably smaller than the\namount of data a program may have to deal with?\n\nDale\n"},{"id":"242882","messageId":"alpine.DEB.2.02.1405281121200.32611@nftneq.ynat.uz","threadId":"36765","inReplyTo":"201405281815.s4SIF5hF025886@hobgoblin.ariadne.com","subject":"Re: Git chokes on large file","fromName":"David Lang","fromEmail":"david@lang.hm","sentAt":"2014-05-28T18:23:10Z","receivedAt":"2014-05-28T18:23:10Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Wed, 28 May 2014, Dale R. Worley wrote:\n\n>> From: Duy Nguyen <pclouds@gmail.com>\n>\n>> I don't know how many commands are hit by this. If you have time and\n>> gdb, please put a break point in die_builtin() function and send\n>> backtraces for those that fail. You could speed up the process by\n>> creating a smaller file and set the environment variable\n>> GIT_ALLOC_LIMIT (in kilobytes) to a number lower than that size. If\n>> git attempts to allocate a block larger than that limit it'll die.\n>\n> I don't use Git enough to exercise it well.  And there are dozens of\n> commands with hundreds of options.\n>\n> As someone else has noted, if I run 'git commit -q --no-status', it\n> doesn't crash.\n>\n> It seems that much of Git was coded under the assumption that any file\n> could always be held entirely in RAM.  Who made that mistake?  Are\n> people so out of touch with reality?\n\nGit was designed to track source code, there are warts that show up in the \nimplementation when you use individual files >4GB\n\nsuch files tend to also not diff well. git-annex and other offshoots hae methods \nboled on that handle such large files better than core git does.\n\nDavid Lang\n"},{"id":"242887","messageId":"201405281847.s4SIlW5K027160@hobgoblin.ariadne.com","threadId":"36765","inReplyTo":"alpine.DEB.2.02.1405281121200.32611@nftneq.ynat.uz","subject":"Re: Git chokes on large file","fromName":"Dale R. Worley","fromEmail":"worley@alum.mit.edu","sentAt":"2014-05-28T18:47:32Z","receivedAt":"2014-05-28T18:47:32Z","isPatch":false,"sender":{"key":"worley@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/19911107?v=4"},"body":"> From: David Lang <david@lang.hm>\n\n> Git was designed to track source code, there are warts that show up\n> in the implementation when you use individual files >4GB\n\nI'd expect that if you want to deal with files over 100k, you should\nassume that it doesn't all fit in memory.\n\nDale\n"},{"id":"242890","messageId":"xmqqmwe1d991.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"alpine.DEB.2.02.1405281121200.32611@nftneq.ynat.uz","subject":"Re: Git chokes on large file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-28T18:54:34Z","receivedAt":"2014-05-28T18:54:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Lang <david@lang.hm> writes:\n\n> On Wed, 28 May 2014, Dale R. Worley wrote:\n>\n>> It seems that much of Git was coded under the assumption that any file\n>> could always be held entirely in RAM.  Who made that mistake?  Are\n>> people so out of touch with reality?\n>\n> Git was designed to track source code, there are warts that show up in\n> the implementation when you use individual files >4GB\n>\n> such files tend to also not diff well. git-annex and other offshoots\n> hae methods boled on that handle such large files better than core git\n> does.\n\nVery well explained, but perhaps you went a bit too far, I am\nafraid.\n\nThe fact that our primary focus being the source code does not at\nall mean that we are not interested to enhance the system to also\ncater to those who want to put materials that are traditionally\nconsidered non-source to it, now that we have become fairly good at\ndoing the source code.\n"},{"id":"242891","messageId":"alpine.DEB.2.02.1405281200570.32611@nftneq.ynat.uz","threadId":"36765","inReplyTo":"201405281847.s4SIlW5K027160@hobgoblin.ariadne.com","subject":"Re: Git chokes on large file","fromName":"David Lang","fromEmail":"david@lang.hm","sentAt":"2014-05-28T19:05:00Z","receivedAt":"2014-05-28T19:05:00Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Wed, 28 May 2014, Dale R. Worley wrote:\n\n>> From: David Lang <david@lang.hm>\n>\n>> Git was designed to track source code, there are warts that show up\n>> in the implementation when you use individual files >4GB\n>\n> I'd expect that if you want to deal with files over 100k, you should\n> assume that it doesn't all fit in memory.\n\nwell, as others noted, the problem is actually caused by doing the diffs, and \nthat is something that is a very common thing to do with source code.\n\nAnd I would assume that files of several MB would be able to fit in memory \n(again, this was assumed to be for development, and compilers take a lot of ram \nto run, so having enough ram to hold any individual source file while the \ncompiler is _not_ using ram doesn't seem likely to be a problem)\n\nDavid Lang\n"},{"id":"242892","messageId":"alpine.DEB.2.02.1405281205410.32611@nftneq.ynat.uz","threadId":"36765","inReplyTo":"xmqqmwe1d991.fsf@gitster.dls.corp.google.com","subject":"Re: Git chokes on large file","fromName":"David Lang","fromEmail":"david@lang.hm","sentAt":"2014-05-28T19:09:28Z","receivedAt":"2014-05-28T19:09:28Z","isPatch":false,"sender":{"key":"david@lang.hm","avatar":null},"body":"On Wed, 28 May 2014, Junio C Hamano wrote:\n\n> David Lang <david@lang.hm> writes:\n>\n>> On Wed, 28 May 2014, Dale R. Worley wrote:\n>>\n>>> It seems that much of Git was coded under the assumption that any file\n>>> could always be held entirely in RAM.  Who made that mistake?  Are\n>>> people so out of touch with reality?\n>>\n>> Git was designed to track source code, there are warts that show up in\n>> the implementation when you use individual files >4GB\n>>\n>> such files tend to also not diff well. git-annex and other offshoots\n>> hae methods boled on that handle such large files better than core git\n>> does.\n>\n> Very well explained, but perhaps you went a bit too far, I am\n> afraid.\n>\n> The fact that our primary focus being the source code does not at\n> all mean that we are not interested to enhance the system to also\n> cater to those who want to put materials that are traditionally\n> considered non-source to it, now that we have become fairly good at\n> doing the source code.\n\nCorrect, I didn't mean to imply that git is only for source files, just noting \nit's origional purpose.\n\nnow that there are multiple different add-ons for git to handle large files in \ndifferent ways, I'm watching to see what can get folded back into the core.\n\nDavid Lang\n"},{"id":"242939","messageId":"1401368227-14469-1-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"CACsJy8BM1f1pJPzGPf--a-kUim6wyX+Mr1AfMupY3mpREY+8DA@mail.gmail.com","subject":"[PATCH 1/4] wrapper.c: introduce gentle xmallocz that does not die()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-29T12:57:04Z","receivedAt":"2014-05-29T12:57:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n git-compat-util.h |  1 +\n wrapper.c         | 68 ++++++++++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 53 insertions(+), 16 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f6d3a46..f23e4e4 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -524,6 +524,7 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);\n extern char *xstrdup(const char *str);\n extern void *xmalloc(size_t size);\n extern void *xmallocz(size_t size);\n+extern void *xmallocz_gentle(size_t size);\n extern void *xmemdupz(const void *data, size_t len);\n extern char *xstrndup(const char *str, size_t len);\n extern void *xrealloc(void *ptr, size_t size);\ndiff --git a/wrapper.c b/wrapper.c\nindex 0cc5636..7ab9a98 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -9,16 +9,23 @@ static void do_nothing(size_t size)\n \n static void (*try_to_free_routine)(size_t size) = do_nothing;\n \n-static void memory_limit_check(size_t size)\n+static int memory_limit_check(size_t size, int gentle)\n {\n \tstatic int limit = -1;\n \tif (limit == -1) {\n \t\tconst char *env = getenv(\"GIT_ALLOC_LIMIT\");\n \t\tlimit = env ? atoi(env) * 1024 : 0;\n \t}\n-\tif (limit && size > limit)\n-\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n-\t\t    (intmax_t)size, limit);\n+\tif (limit && size > limit) {\n+\t\tif (gentle) {\n+\t\t\terror(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t      (intmax_t)size, limit);\n+\t\t\treturn -1;\n+\t\t} else\n+\t\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t    (intmax_t)size, limit);\n+\t}\n+\treturn 0;\n }\n \n try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n@@ -42,11 +49,12 @@ char *xstrdup(const char *str)\n \treturn ret;\n }\n \n-void *xmalloc(size_t size)\n+static void *do_xmalloc(size_t size, int gentle)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tif (memory_limit_check(size, gentle))\n+\t\treturn NULL;\n \tret = malloc(size);\n \tif (!ret && !size)\n \t\tret = malloc(1);\n@@ -55,9 +63,16 @@ void *xmalloc(size_t size)\n \t\tret = malloc(size);\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n-\t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t    (unsigned long)size);\n+\t\tif (!ret) {\n+\t\t\tif (!gentle)\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t    (unsigned long)size);\n+\t\t\telse {\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t      (unsigned long)size);\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t}\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n@@ -65,16 +80,37 @@ void *xmalloc(size_t size)\n \treturn ret;\n }\n \n-void *xmallocz(size_t size)\n+void *xmalloc(size_t size)\n+{\n+\treturn do_xmalloc(size, 0);\n+}\n+\n+static void *do_xmallocz(size_t size, int gentle)\n {\n \tvoid *ret;\n-\tif (unsigned_add_overflows(size, 1))\n-\t\tdie(\"Data too large to fit into virtual memory space.\");\n-\tret = xmalloc(size + 1);\n-\t((char*)ret)[size] = 0;\n+\tif (unsigned_add_overflows(size, 1)) {\n+\t\tif (gentle) {\n+\t\t\terror(\"Data too large to fit into virtual memory space.\");\n+\t\t\treturn NULL;\n+\t\t} else\n+\t\t\tdie(\"Data too large to fit into virtual memory space.\");\n+\t}\n+\tret = do_xmalloc(size + 1, gentle);\n+\tif (ret)\n+\t\t((char*)ret)[size] = 0;\n \treturn ret;\n }\n \n+void *xmallocz(size_t size)\n+{\n+\treturn do_xmallocz(size, 0);\n+}\n+\n+void *xmallocz_gentle(size_t size)\n+{\n+\treturn do_xmallocz(size, 1);\n+}\n+\n /*\n  * xmemdupz() allocates (len + 1) bytes of memory, duplicates \"len\" bytes of\n  * \"data\" to the allocated memory, zero terminates the allocated memory,\n@@ -96,7 +132,7 @@ void *xrealloc(void *ptr, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tmemory_limit_check(size, 0);\n \tret = realloc(ptr, size);\n \tif (!ret && !size)\n \t\tret = realloc(ptr, 1);\n@@ -115,7 +151,7 @@ void *xcalloc(size_t nmemb, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size * nmemb);\n+\tmemory_limit_check(size * nmemb, 0);\n \tret = calloc(nmemb, size);\n \tif (!ret && (!nmemb || !size))\n \t\tret = calloc(1, 1);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"242940","messageId":"1401368227-14469-2-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1401368227-14469-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/4] fsck: do not die when not enough memory to examine a pack entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-29T12:57:05Z","receivedAt":"2014-05-29T12:57:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"fsck is a tool that error() is more preferred than die(), but many\nfunctions embed die() inside beyond fsck's control.\nunpack_compressed_entry()'s using xmallocz is such a function,\ntriggered from verify_packfile() -> unpack_entry(). Make it use\nxmallocz_gentle() instead.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c      | 4 +++-\n t/t1050-large.sh | 5 +++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3e9f55f..8ad906a 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1921,7 +1921,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n \tgit_zstream stream;\n \tunsigned char *buffer, *in;\n \n-\tbuffer = xmallocz(size);\n+\tbuffer = xmallocz_gentle(size);\n+\tif (!buffer)\n+\t\treturn NULL;\n \tmemset(&stream, 0, sizeof(stream));\n \tstream.next_out = buffer;\n \tstream.avail_out = size + 1;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex fd10528..333909b 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -163,4 +163,9 @@ test_expect_success 'zip achiving, deflate' '\n \tgit archive --format=zip HEAD >/dev/null\n '\n \n+test_expect_success 'fsck' '\n+\ttest_must_fail git fsck 2>err &&\n+\tgrep \"attempting to allocate .* over limit\" err\n+'\n+\n test_done\n-- \n1.9.1.346.ga2b5940\n"},{"id":"242941","messageId":"1401368227-14469-3-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1401368227-14469-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/4] diff.c: allow to pass more flags to diff_populate_filespec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-29T12:57:06Z","receivedAt":"2014-05-29T12:57:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c            | 13 +++++++------\n diffcore-rename.c |  6 ++++--\n diffcore.h        |  3 ++-\n 3 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex f72769a..54281cb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -373,7 +373,7 @@ static unsigned long diff_filespec_size(struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one))\n \t\treturn 0;\n-\tdiff_populate_filespec(one, 1);\n+\tdiff_populate_filespec(one, DIFF_POPULATE_SIZE_ONLY);\n \treturn one->size;\n }\n \n@@ -1907,11 +1907,11 @@ static void show_dirstat(struct diff_options *options)\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t\tdiff_free_filespec_data(p->two);\n \t\t} else if (DIFF_FILE_VALID(p->one)) {\n-\t\t\tdiff_populate_filespec(p->one, 1);\n+\t\t\tdiff_populate_filespec(p->one, DIFF_POPULATE_SIZE_ONLY);\n \t\t\tcopied = added = 0;\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t} else if (DIFF_FILE_VALID(p->two)) {\n-\t\t\tdiff_populate_filespec(p->two, 1);\n+\t\t\tdiff_populate_filespec(p->two, DIFF_POPULATE_SIZE_ONLY);\n \t\t\tcopied = 0;\n \t\t\tadded = p->two->size;\n \t\t\tdiff_free_filespec_data(p->two);\n@@ -2664,8 +2664,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n  * grab the data for the blob (or file) for our own in-core comparison.\n  * diff_filespec has data and size fields for this purpose.\n  */\n-int diff_populate_filespec(struct diff_filespec *s, int size_only)\n+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n {\n+\tint size_only = flags & DIFF_POPULATE_SIZE_ONLY;\n \tint err = 0;\n \t/*\n \t * demote FAIL to WARN to allow inspecting the situation\n@@ -4695,8 +4696,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)\n \t    !DIFF_FILE_VALID(p->two) ||\n \t    (p->one->sha1_valid && p->two->sha1_valid) ||\n \t    (p->one->mode != p->two->mode) ||\n-\t    diff_populate_filespec(p->one, 1) ||\n-\t    diff_populate_filespec(p->two, 1) ||\n+\t    diff_populate_filespec(p->one, DIFF_POPULATE_SIZE_ONLY) ||\n+\t    diff_populate_filespec(p->two, DIFF_POPULATE_SIZE_ONLY) ||\n \t    (p->one->size != p->two->size) ||\n \t    !diff_filespec_is_identical(p->one, p->two)) /* (2) */\n \t\tp->skip_stat_unmatch_result = 1;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 749a35d..8437917 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,\n \t * is a possible size - we really should have a flag to\n \t * say whether the size is valid or not!)\n \t */\n-\tif (!src->cnt_data && diff_populate_filespec(src, 1))\n+\tif (!src->cnt_data &&\n+\t    diff_populate_filespec(src, DIFF_POPULATE_SIZE_ONLY))\n \t\treturn 0;\n-\tif (!dst->cnt_data && diff_populate_filespec(dst, 1))\n+\tif (!dst->cnt_data &&\n+\t    diff_populate_filespec(dst, DIFF_POPULATE_SIZE_ONLY))\n \t\treturn 0;\n \n \tmax_size = ((src->size > dst->size) ? src->size : dst->size);\ndiff --git a/diffcore.h b/diffcore.h\nindex c876dac..a186d7c 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);\n extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n-extern int diff_populate_filespec(struct diff_filespec *, int);\n+#define DIFF_POPULATE_SIZE_ONLY 1\n+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\n extern int diff_filespec_is_binary(struct diff_filespec *);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"242942","messageId":"1401368227-14469-4-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1401368227-14469-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-29T12:57:07Z","receivedAt":"2014-05-29T12:57:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Too large files may lead to failure to allocate memory. If it happens\nhere, it could impact quite a few commands that involve\ndiff. Moreover, too large files are inefficient to compare anyway (and\nmost likely non-text), so mark them binary and skip looking at their\ncontent.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c           | 26 ++++++++++++++++++--------\n diffcore.h       |  1 +\n t/t1050-large.sh |  4 ++++\n 3 files changed, 23 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 54281cb..0a2f865 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \t\t\tone->is_binary = one->driver->binary;\n \t\telse {\n \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\t\t\tdiff_populate_filespec(one, 0);\n-\t\t\tif (one->data)\n+\t\t\t\tdiff_populate_filespec(one, DIFF_POPULATE_IS_BINARY);\n+\t\t\tif (one->is_binary == -1 && one->data)\n \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n \t\t\t\t\t\tone->size);\n \t\t\tif (one->is_binary == -1)\n@@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n+\t\tif ((flags & DIFF_POPULATE_IS_BINARY) &&\n+\t\t    s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\ts->is_binary = 1;\n+\t\t\treturn 0;\n+\t\t}\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n@@ -2742,16 +2747,21 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t}\n \telse {\n \t\tenum object_type type;\n-\t\tif (size_only) {\n+\t\tif (size_only || (flags & DIFF_POPULATE_IS_BINARY)) {\n \t\t\ttype = sha1_object_info(s->sha1, &s->size);\n \t\t\tif (type < 0)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t} else {\n-\t\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n-\t\t\tif (!s->data)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t\ts->should_free = 1;\n+\t\t\tif (size_only)\n+\t\t\t\treturn 0;\n+\t\t\tif (s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\t\ts->is_binary = 1;\n+\t\t\t\treturn 0;\n+\t\t\t}\n \t\t}\n+\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n+\t\tif (!s->data)\n+\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n+\t\ts->should_free = 1;\n \t}\n \treturn 0;\n }\ndiff --git a/diffcore.h b/diffcore.h\nindex a186d7c..e7760d9 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n #define DIFF_POPULATE_SIZE_ONLY 1\n+#define DIFF_POPULATE_IS_BINARY 2\n extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 333909b..4d922e2 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '\n \tgit diff --raw HEAD^\n '\n \n+test_expect_success 'diff --stat' '\n+\tgit diff --stat HEAD^ HEAD\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n1.9.1.346.ga2b5940\n"},{"id":"242974","messageId":"201405291912.s4TJC2Wr028094@hobgoblin.ariadne.com","threadId":"36765","inReplyTo":"alpine.DEB.2.02.1405281200570.32611@nftneq.ynat.uz","subject":"Re: Git chokes on large file","fromName":"Dale R. Worley","fromEmail":"worley@alum.mit.edu","sentAt":"2014-05-29T19:12:02Z","receivedAt":"2014-05-29T19:12:02Z","isPatch":false,"sender":{"key":"worley@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/19911107?v=4"},"body":"> From: David Lang <david@lang.hm>\n\n> well, as others noted, the problem is actually caused by doing the diffs, and \n> that is something that is a very common thing to do with source code.\n\nTo some degree, my attitude comes from When I Was A Boy, when you got\n16k for both your bytecode and your data, so you never kept more than\none line of a file in memory unless you had to.\n\nRegardless of that, the problem with \"git fsck\" is not due to doing\ndiffs, and \"git commit\" by default does diffs even if you don't ask\nfor them, so the observed problems cannot be subsumed under the label\n\"you asked for a diff of a file that can't be diffed\".\n\n> And I would assume that files of several MB would be able to fit in\n> memory (again, this was assumed to be for development, and compilers\n> take a lot of ram to run, so having enough ram to hold any\n> individual source file while the compiler is _not_ using ram doesn't\n> seem likely to be a problem)\n\nAt least the first versions of GCC only kept one function (and the\nglobal symbol table) in memory at once, so you could compile a source\nfile that was larger than the available memory.\n\nIn any case, if Git is no longer limited to handling source files, it\nneeds to be updated so it can handle large files.\n\nDale\n"},{"id":"244639","messageId":"1403180845.10052.16.camel@thomas-debian-x64","threadId":"36765","inReplyTo":"1401368227-14469-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2014-06-19T12:27:25Z","receivedAt":"2014-06-19T12:27:25Z","isPatch":true,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am Donnerstag, den 29.05.2014, 19:57 +0700 schrieb Nguyễn Thái Ngọc Duy:\n\nHi,\n\nsorry for chiming in so late.\n\nI've just played around with patch 3 and 4 of that series.\nAnd I like it very much as I work often with large files so any further \nenhancement in that area is really nice.\n\n(see comments below)\n\n> Too large files may lead to failure to allocate memory. If it happens\n> here, it could impact quite a few commands that involve\n> diff. Moreover, too large files are inefficient to compare anyway (and\n> most likely non-text), so mark them binary and skip looking at their\n> content.\n> \n> Noticed-by: Dale R. Worley <worley@alum.mit.edu>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  diff.c           | 26 ++++++++++++++++++--------\n>  diffcore.h       |  1 +\n>  t/t1050-large.sh |  4 ++++\n>  3 files changed, 23 insertions(+), 8 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 54281cb..0a2f865 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n>  \t\t\tone->is_binary = one->driver->binary;\n>  \t\telse {\n>  \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n> -\t\t\t\tdiff_populate_filespec(one, 0);\n> -\t\t\tif (one->data)\n> +\t\t\t\tdiff_populate_filespec(one, DIFF_POPULATE_IS_BINARY);\n> +\t\t\tif (one->is_binary == -1 && one->data)\n>  \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n>  \t\t\t\t\t\tone->size);\n>  \t\t\tif (one->is_binary == -1)\n> @@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>  \t\t}\n>  \t\tif (size_only)\n>  \t\t\treturn 0;\n> +\t\tif ((flags & DIFF_POPULATE_IS_BINARY) &&\n> +\t\t    s->size > big_file_threshold && s->is_binary == -1) {\n> +\t\t\ts->is_binary = 1;\n> +\t\t\treturn 0;\n> +\t\t}\n\nWhy do you check for s->is_binary == -1 here? I think it does not matter\nwhat s_is_binary says here.\n\n>  \t\tfd = open(s->path, O_RDONLY);\n>  \t\tif (fd < 0)\n>  \t\t\tgoto err_empty;\n> @@ -2742,16 +2747,21 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>  \t}\n>  \telse {\n>  \t\tenum object_type type;\n> -\t\tif (size_only) {\n> +\t\tif (size_only || (flags & DIFF_POPULATE_IS_BINARY)) {\n>  \t\t\ttype = sha1_object_info(s->sha1, &s->size);\n>  \t\t\tif (type < 0)\n>  \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n> -\t\t} else {\n> -\t\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n> -\t\t\tif (!s->data)\n> -\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n> -\t\t\ts->should_free = 1;\n> +\t\t\tif (size_only)\n> +\t\t\t\treturn 0;\n> +\t\t\tif (s->size > big_file_threshold && s->is_binary == -1) {\nsame as above.\n> +\t\t\t\ts->is_binary = 1;\n> +\t\t\t\treturn 0;\n> +\t\t\t}\n>  \t\t}\n> +\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n> +\t\tif (!s->data)\n> +\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n> +\t\ts->should_free = 1;\n>  \t}\n>  \treturn 0;\n>  }\n> diff --git a/diffcore.h b/diffcore.h\n> index a186d7c..e7760d9 100644\n> --- a/diffcore.h\n> +++ b/diffcore.h\n> @@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n>  \t\t\t  int, unsigned short);\n>  \n>  #define DIFF_POPULATE_SIZE_ONLY 1\n> +#define DIFF_POPULATE_IS_BINARY 2\n>  extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n>  extern void diff_free_filespec_data(struct diff_filespec *);\n>  extern void diff_free_filespec_blob(struct diff_filespec *);\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index 333909b..4d922e2 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '\n>  \tgit diff --raw HEAD^\n>  '\n>  \n> +test_expect_success 'diff --stat' '\n> +\tgit diff --stat HEAD^ HEAD\n> +'\n> +\n>  test_expect_success 'hash-object' '\n>  \tgit hash-object large1\n>  '\n\nI would also add a note to the documentation e. g:\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 9f467d3..7a2f27d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -499,7 +499,8 @@ core.bigFileThreshold::\n        Files larger than this size are stored deflated, without\n        attempting delta compression.  Storing large files without\n        delta compression avoids excessive memory usage, at the\n-       slight expense of increased disk usage.\n+       slight expense of increased disk usage.  Additionally files\n+       larger than this size are allways treated as binary.\n +\n Default is 512 MiB on all platforms.  This should be reasonable\n for most projects as source code and other text files can still\n\nThomas\n"},{"id":"244868","messageId":"CACsJy8A5StEv4O64rnd39+1jMNWiaAv4oOd0+Yko_JPuk6EYZw@mail.gmail.com","threadId":"36765","inReplyTo":"1403180845.10052.16.camel@thomas-debian-x64","subject":"Re: [PATCH 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-23T12:18:37Z","receivedAt":"2014-06-23T12:18:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 19, 2014 at 7:27 PM, Thomas Braun\n<thomas.braun@virtuell-zuhause.de> wrote:\n>> @@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>>               }\n>>               if (size_only)\n>>                       return 0;\n>> +             if ((flags & DIFF_POPULATE_IS_BINARY) &&\n>> +                 s->size > big_file_threshold && s->is_binary == -1) {\n>> +                     s->is_binary = 1;\n>> +                     return 0;\n>> +             }\n>\n> Why do you check for s->is_binary == -1 here? I think it does not matter\n> what s_is_binary says here.\n\nIf some .gitattributes to mark one file not-binary, we should respect\nthat, I think. Same for below too.\n\n> I would also add a note to the documentation e. g:\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 9f467d3..7a2f27d 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -499,7 +499,8 @@ core.bigFileThreshold::\n>         Files larger than this size are stored deflated, without\n>         attempting delta compression.  Storing large files without\n>         delta compression avoids excessive memory usage, at the\n> -       slight expense of increased disk usage.\n> +       slight expense of increased disk usage.  Additionally files\n> +       larger than this size are allways treated as binary.\n>  +\n>  Default is 512 MiB on all platforms.  This should be reasonable\n>  for most projects as source code and other text files can still\n\nThanks. Will do. Sorry a little busy these days and could not reply earlier.\n-- \nDuy\n"},{"id":"244891","messageId":"53A87E28.8050903@virtuell-zuhause.de","threadId":"36765","inReplyTo":"CACsJy8A5StEv4O64rnd39+1jMNWiaAv4oOd0+Yko_JPuk6EYZw@mail.gmail.com","subject":"Re: [PATCH 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2014-06-23T19:21:12Z","receivedAt":"2014-06-23T19:21:12Z","isPatch":true,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am 23.06.2014 14:18, schrieb Duy Nguyen:\n> On Thu, Jun 19, 2014 at 7:27 PM, Thomas Braun\n> <thomas.braun@virtuell-zuhause.de> wrote:\n>>> @@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>>>               }\n>>>               if (size_only)\n>>>                       return 0;\n>>> +             if ((flags & DIFF_POPULATE_IS_BINARY) &&\n>>> +                 s->size > big_file_threshold && s->is_binary == -1) {\n>>> +                     s->is_binary = 1;\n>>> +                     return 0;\n>>> +             }\n>>\n>> Why do you check for s->is_binary == -1 here? I think it does not matter\n>> what s_is_binary says here.\n> \n> If some .gitattributes to mark one file not-binary, we should respect\n> that, I think. Same for below too.\n\nReading diffcore.h I thought is_binary being -1 means it is not yet\ndecided if it is binary or text.\nRespecting .gitattributes is obviously a good thing :)\n"},{"id":"244923","messageId":"1403610336-27761-1-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1401368227-14469-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/4] wrapper.c: introduce gentle xmallocz that does not die()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-24T11:45:33Z","receivedAt":"2014-06-24T11:45:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n git-compat-util.h |  1 +\n wrapper.c         | 68 ++++++++++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 53 insertions(+), 16 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b6f03b3..7eee0f2 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -540,6 +540,7 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);\n extern char *xstrdup(const char *str);\n extern void *xmalloc(size_t size);\n extern void *xmallocz(size_t size);\n+extern void *xmallocz_gentle(size_t size);\n extern void *xmemdupz(const void *data, size_t len);\n extern char *xstrndup(const char *str, size_t len);\n extern void *xrealloc(void *ptr, size_t size);\ndiff --git a/wrapper.c b/wrapper.c\nindex bc1bfb8..06601c4 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -9,16 +9,23 @@ static void do_nothing(size_t size)\n \n static void (*try_to_free_routine)(size_t size) = do_nothing;\n \n-static void memory_limit_check(size_t size)\n+static int memory_limit_check(size_t size, int gentle)\n {\n \tstatic int limit = -1;\n \tif (limit == -1) {\n \t\tconst char *env = getenv(\"GIT_ALLOC_LIMIT\");\n \t\tlimit = env ? atoi(env) * 1024 : 0;\n \t}\n-\tif (limit && size > limit)\n-\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n-\t\t    (intmax_t)size, limit);\n+\tif (limit && size > limit) {\n+\t\tif (gentle) {\n+\t\t\terror(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t      (intmax_t)size, limit);\n+\t\t\treturn -1;\n+\t\t} else\n+\t\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t    (intmax_t)size, limit);\n+\t}\n+\treturn 0;\n }\n \n try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n@@ -42,11 +49,12 @@ char *xstrdup(const char *str)\n \treturn ret;\n }\n \n-void *xmalloc(size_t size)\n+static void *do_xmalloc(size_t size, int gentle)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tif (memory_limit_check(size, gentle))\n+\t\treturn NULL;\n \tret = malloc(size);\n \tif (!ret && !size)\n \t\tret = malloc(1);\n@@ -55,9 +63,16 @@ void *xmalloc(size_t size)\n \t\tret = malloc(size);\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n-\t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t    (unsigned long)size);\n+\t\tif (!ret) {\n+\t\t\tif (!gentle)\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t    (unsigned long)size);\n+\t\t\telse {\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t      (unsigned long)size);\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t}\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n@@ -65,16 +80,37 @@ void *xmalloc(size_t size)\n \treturn ret;\n }\n \n-void *xmallocz(size_t size)\n+void *xmalloc(size_t size)\n+{\n+\treturn do_xmalloc(size, 0);\n+}\n+\n+static void *do_xmallocz(size_t size, int gentle)\n {\n \tvoid *ret;\n-\tif (unsigned_add_overflows(size, 1))\n-\t\tdie(\"Data too large to fit into virtual memory space.\");\n-\tret = xmalloc(size + 1);\n-\t((char*)ret)[size] = 0;\n+\tif (unsigned_add_overflows(size, 1)) {\n+\t\tif (gentle) {\n+\t\t\terror(\"Data too large to fit into virtual memory space.\");\n+\t\t\treturn NULL;\n+\t\t} else\n+\t\t\tdie(\"Data too large to fit into virtual memory space.\");\n+\t}\n+\tret = do_xmalloc(size + 1, gentle);\n+\tif (ret)\n+\t\t((char*)ret)[size] = 0;\n \treturn ret;\n }\n \n+void *xmallocz(size_t size)\n+{\n+\treturn do_xmallocz(size, 0);\n+}\n+\n+void *xmallocz_gentle(size_t size)\n+{\n+\treturn do_xmallocz(size, 1);\n+}\n+\n /*\n  * xmemdupz() allocates (len + 1) bytes of memory, duplicates \"len\" bytes of\n  * \"data\" to the allocated memory, zero terminates the allocated memory,\n@@ -96,7 +132,7 @@ void *xrealloc(void *ptr, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tmemory_limit_check(size, 0);\n \tret = realloc(ptr, size);\n \tif (!ret && !size)\n \t\tret = realloc(ptr, 1);\n@@ -115,7 +151,7 @@ void *xcalloc(size_t nmemb, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size * nmemb);\n+\tmemory_limit_check(size * nmemb, 0);\n \tret = calloc(nmemb, size);\n \tif (!ret && (!nmemb || !size))\n \t\tret = calloc(1, 1);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"244922","messageId":"1403610336-27761-2-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1403610336-27761-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/4] fsck: do not die when not enough memory to examine a pack entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-24T11:45:34Z","receivedAt":"2014-06-24T11:45:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"fsck is a tool that error() is more preferred than die(), but many\nfunctions embed die() inside beyond fsck's control.\nunpack_compressed_entry()'s using xmallocz is such a function,\ntriggered from verify_packfile() -> unpack_entry(). Make it use\nxmallocz_gentle() instead.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c      | 4 +++-\n t/t1050-large.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 34d527f..eb69c78 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1925,7 +1925,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n \tgit_zstream stream;\n \tunsigned char *buffer, *in;\n \n-\tbuffer = xmallocz(size);\n+\tbuffer = xmallocz_gentle(size);\n+\tif (!buffer)\n+\t\treturn NULL;\n \tmemset(&stream, 0, sizeof(stream));\n \tstream.next_out = buffer;\n \tstream.avail_out = size + 1;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex aea4936..5642f84 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '\n \tgit archive --format=zip HEAD >/dev/null\n '\n \n+test_expect_success 'fsck' '\n+\ttest_must_fail git fsck 2>err &&\n+\tn=$(grep \"error: attempting to allocate .* over limit\" err | wc -l) &&\n+\ttest \"$n\" -gt 1\n+'\n+\n test_done\n-- \n1.9.1.346.ga2b5940\n"},{"id":"244924","messageId":"1403610336-27761-3-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1403610336-27761-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/4] diff.c: allow to pass more flags to diff_populate_filespec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-24T11:45:35Z","receivedAt":"2014-06-24T11:45:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c            | 13 +++++++------\n diffcore-rename.c |  6 ++++--\n diffcore.h        |  3 ++-\n 3 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex bba9a55..a489540 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -373,7 +373,7 @@ static unsigned long diff_filespec_size(struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one))\n \t\treturn 0;\n-\tdiff_populate_filespec(one, 1);\n+\tdiff_populate_filespec(one, DIFF_POPULATE_SIZE_ONLY);\n \treturn one->size;\n }\n \n@@ -1907,11 +1907,11 @@ static void show_dirstat(struct diff_options *options)\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t\tdiff_free_filespec_data(p->two);\n \t\t} else if (DIFF_FILE_VALID(p->one)) {\n-\t\t\tdiff_populate_filespec(p->one, 1);\n+\t\t\tdiff_populate_filespec(p->one, DIFF_POPULATE_SIZE_ONLY);\n \t\t\tcopied = added = 0;\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t} else if (DIFF_FILE_VALID(p->two)) {\n-\t\t\tdiff_populate_filespec(p->two, 1);\n+\t\t\tdiff_populate_filespec(p->two, DIFF_POPULATE_SIZE_ONLY);\n \t\t\tcopied = 0;\n \t\t\tadded = p->two->size;\n \t\t\tdiff_free_filespec_data(p->two);\n@@ -2664,8 +2664,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n  * grab the data for the blob (or file) for our own in-core comparison.\n  * diff_filespec has data and size fields for this purpose.\n  */\n-int diff_populate_filespec(struct diff_filespec *s, int size_only)\n+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n {\n+\tint size_only = flags & DIFF_POPULATE_SIZE_ONLY;\n \tint err = 0;\n \t/*\n \t * demote FAIL to WARN to allow inspecting the situation\n@@ -4693,8 +4694,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)\n \t    !DIFF_FILE_VALID(p->two) ||\n \t    (p->one->sha1_valid && p->two->sha1_valid) ||\n \t    (p->one->mode != p->two->mode) ||\n-\t    diff_populate_filespec(p->one, 1) ||\n-\t    diff_populate_filespec(p->two, 1) ||\n+\t    diff_populate_filespec(p->one, DIFF_POPULATE_SIZE_ONLY) ||\n+\t    diff_populate_filespec(p->two, DIFF_POPULATE_SIZE_ONLY) ||\n \t    (p->one->size != p->two->size) ||\n \t    !diff_filespec_is_identical(p->one, p->two)) /* (2) */\n \t\tp->skip_stat_unmatch_result = 1;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 749a35d..8437917 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,\n \t * is a possible size - we really should have a flag to\n \t * say whether the size is valid or not!)\n \t */\n-\tif (!src->cnt_data && diff_populate_filespec(src, 1))\n+\tif (!src->cnt_data &&\n+\t    diff_populate_filespec(src, DIFF_POPULATE_SIZE_ONLY))\n \t\treturn 0;\n-\tif (!dst->cnt_data && diff_populate_filespec(dst, 1))\n+\tif (!dst->cnt_data &&\n+\t    diff_populate_filespec(dst, DIFF_POPULATE_SIZE_ONLY))\n \t\treturn 0;\n \n \tmax_size = ((src->size > dst->size) ? src->size : dst->size);\ndiff --git a/diffcore.h b/diffcore.h\nindex c876dac..a186d7c 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);\n extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n-extern int diff_populate_filespec(struct diff_filespec *, int);\n+#define DIFF_POPULATE_SIZE_ONLY 1\n+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\n extern int diff_filespec_is_binary(struct diff_filespec *);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"244925","messageId":"1403610336-27761-4-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1403610336-27761-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-24T11:45:36Z","receivedAt":"2014-06-24T11:45:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Too large files may lead to failure to allocate memory. If it happens\nhere, it could impact quite a few commands that involve\ndiff. Moreover, too large files are inefficient to compare anyway (and\nmost likely non-text), so mark them binary and skip looking at their\ncontent.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt        |  3 ++-\n Documentation/gitattributes.txt |  4 ++--\n diff.c                          | 26 ++++++++++++++++++--------\n diffcore.h                      |  1 +\n t/t1050-large.sh                |  4 ++++\n 5 files changed, 27 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 9f467d3..a865850 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -499,7 +499,8 @@ core.bigFileThreshold::\n \tFiles larger than this size are stored deflated, without\n \tattempting delta compression.  Storing large files without\n \tdelta compression avoids excessive memory usage, at the\n-\tslight expense of increased disk usage.\n+\tslight expense of increased disk usage. Additionally files\n+\tlarger than this size are allways treated as binary.\n +\n Default is 512 MiB on all platforms.  This should be reasonable\n for most projects as source code and other text files can still\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 643c1ba..9b45bda 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -440,8 +440,8 @@ Unspecified::\n \n \tA path to which the `diff` attribute is unspecified\n \tfirst gets its contents inspected, and if it looks like\n-\ttext, it is treated as text.  Otherwise it would\n-\tgenerate `Binary files differ`.\n+\ttext and is smaller than core.bigFileThreshold, it is treated\n+\tas text. Otherwise it would generate `Binary files differ`.\n \n String::\n \ndiff --git a/diff.c b/diff.c\nindex a489540..7a977aa 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \t\t\tone->is_binary = one->driver->binary;\n \t\telse {\n \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\t\t\tdiff_populate_filespec(one, 0);\n-\t\t\tif (one->data)\n+\t\t\t\tdiff_populate_filespec(one, DIFF_POPULATE_IS_BINARY);\n+\t\t\tif (one->is_binary == -1 && one->data)\n \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n \t\t\t\t\t\tone->size);\n \t\t\tif (one->is_binary == -1)\n@@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n+\t\tif ((flags & DIFF_POPULATE_IS_BINARY) &&\n+\t\t    s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\ts->is_binary = 1;\n+\t\t\treturn 0;\n+\t\t}\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n@@ -2742,16 +2747,21 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t}\n \telse {\n \t\tenum object_type type;\n-\t\tif (size_only) {\n+\t\tif (size_only || (flags & DIFF_POPULATE_IS_BINARY)) {\n \t\t\ttype = sha1_object_info(s->sha1, &s->size);\n \t\t\tif (type < 0)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t} else {\n-\t\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n-\t\t\tif (!s->data)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t\ts->should_free = 1;\n+\t\t\tif (size_only)\n+\t\t\t\treturn 0;\n+\t\t\tif (s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\t\ts->is_binary = 1;\n+\t\t\t\treturn 0;\n+\t\t\t}\n \t\t}\n+\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n+\t\tif (!s->data)\n+\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n+\t\ts->should_free = 1;\n \t}\n \treturn 0;\n }\ndiff --git a/diffcore.h b/diffcore.h\nindex a186d7c..e7760d9 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n #define DIFF_POPULATE_SIZE_ONLY 1\n+#define DIFF_POPULATE_IS_BINARY 2\n extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 5642f84..00d2f33 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '\n \tgit diff --raw HEAD^\n '\n \n+test_expect_success 'diff --stat' '\n+\tgit diff --stat HEAD^ HEAD\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n1.9.1.346.ga2b5940\n"},{"id":"245030","messageId":"xmqqegyb5zeh.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1403610336-27761-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-26T17:55:18Z","receivedAt":"2014-06-26T17:55:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Too large files may lead to failure to allocate memory. If it happens\n> here, it could impact quite a few commands that involve\n> diff. Moreover, too large files are inefficient to compare anyway (and\n> most likely non-text), so mark them binary and skip looking at their\n> content.\n> ...\n> diff --git a/diff.c b/diff.c\n> index a489540..7a977aa 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n>  \t\t\tone->is_binary = one->driver->binary;\n>  \t\telse {\n>  \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n> -\t\t\t\tdiff_populate_filespec(one, 0);\n> -\t\t\tif (one->data)\n> +\t\t\t\tdiff_populate_filespec(one, DIFF_POPULATE_IS_BINARY);\n> +\t\t\tif (one->is_binary == -1 && one->data)\n>  \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n>  \t\t\t\t\t\tone->size);\n>  \t\t\tif (one->is_binary == -1)\n\nThe name is misleading and forced me to read it twice before I\nrealized that this is \"populating the is-binary bit\".  It might make\nit a bit better if you renamed it to DIFF_POPULATE_IS_BINARY_BIT or\nperhaps DIFF_POPULATE_CHECK_BINARY or something.  For consistency,\nthe other bit may want to be also renamed from SIZE_ONLY to either\n\n (1) CHECK_SIZE_ONLY\n\n (2) One bit for CHECK_SIZE, another for NO_CONTENTS, and optionally\n     make SIZE_ONLY the union of two\n\nI do not have strong preference either way; the latter may be more\nlogical in that \"not loading contents\" and \"check size\" are sort of\northogonal in that you can later choose to check, without loading\ncontents, only the binary-ness without checking size, but no calles\nthat passes a non-zero flag to the populate-filespec function will\nwant to slurp in the contents in practice, so in that sense we could\ndeclare that the NO_CONENTS bit is implied.\n\nBut more importantly, would this patch actually help?  For one\nthing, this wouldn't (and shouldn't) help if the user wants --binary\ndiff out of us anyway, I suspect, but I wonder what the following\ncodepath in the builtin_diff() function would do:\n\n\t\t...\n\t} else if (!DIFF_OPT_TST(o, TEXT) &&\n\t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n\t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n\t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n\t\t\tdie(\"unable to read files to diff\");\n\t\t/* Quite common confusing case */\n\t\tif (mf1.size == mf2.size &&\n\t\t    !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {\n\t\t\tif (must_show_header)\n\t\t\t\tfprintf(o->file, \"%s\", header.buf);\n\t\t\tgoto free_ab_and_return;\n\t\t}\n\t\tfprintf(o->file, \"%s\", header.buf);\n\t\tstrbuf_reset(&header);\n\t\tif (DIFF_OPT_TST(o, BINARY))\n\t\t\temit_binary_diff(o->file, &mf1, &mf2, line_prefix);\n\t\telse\n\t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n\t\t\t\tline_prefix, lbl[0], lbl[1]);\n\t\to->found_changes = 1;\n\t} else {\n\t\t...\n\nIf we weren't told with --text/-a to force textual output, and\nat least one of the sides is marked as binary (and this patch marks\na large blob as binary unless attributes says otherwise), we still\ncall fill_mmfile() on them to slurp the contents to be compared, no?\n\nAnd before you get to the DIFF_OPT_TST(o, BINARY), you memcmp(3) to\ncheck if the sides are identical, so...\n"},{"id":"245031","messageId":"xmqqa98z5yrg.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1403610336-27761-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 2/4] fsck: do not die when not enough memory to examine a pack entry","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-26T18:09:07Z","receivedAt":"2014-06-26T18:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> fsck is a tool that error() is more preferred than die(), but many\n\n\"more preferred\" without justifying why it is \"more preferred\" is\nnot quite a justification, is it?  Also, an object failing to load\nin-core is not a missing object, so if your aim is to let \"fsck\"\ndiagnose a too-large-to-load object as missing and let it continue,\nI do not know if it is \"more preferred\" in the first place.  Adding\na \"too large--cannot check\" bin of objects may be needed for it to\nbe useful.  Also, we might need to give at the end \"oh by the way,\nbecause we couldn't read some objects to even determine its type,\nthe unreachable report from this fsck run is totally useless.\"\n\nThe log message tries to justify that this may be a good thing for\n\"fsck\", but the patch actually tries to change the behaviour of all\ncode paths that try to load an object in-core without considering\nthe ramifications of such a change.  I _think_ all callers should be\nprepared to receive NULL when we encounter a corrupt object (and\notherwise we should fix them), but it is unclear how much audit of\nthe callers (if any) was done to prepare this change.\n\nNot very excited X-<.\n\n> functions embed die() inside beyond fsck's control.\n> unpack_compressed_entry()'s using xmallocz is such a function,\n> triggered from verify_packfile() -> unpack_entry(). Make it use\n> xmallocz_gentle() instead.\n>\n> Noticed-by: Dale R. Worley <worley@alum.mit.edu>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  sha1_file.c      | 4 +++-\n>  t/t1050-large.sh | 6 ++++++\n>  2 files changed, 9 insertions(+), 1 deletion(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 34d527f..eb69c78 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1925,7 +1925,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n>  \tgit_zstream stream;\n>  \tunsigned char *buffer, *in;\n>  \n> -\tbuffer = xmallocz(size);\n> +\tbuffer = xmallocz_gentle(size);\n> +\tif (!buffer)\n> +\t\treturn NULL;\n>  \tmemset(&stream, 0, sizeof(stream));\n>  \tstream.next_out = buffer;\n>  \tstream.avail_out = size + 1;\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index aea4936..5642f84 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '\n>  \tgit archive --format=zip HEAD >/dev/null\n>  '\n>  \n> +test_expect_success 'fsck' '\n> +\ttest_must_fail git fsck 2>err &&\n> +\tn=$(grep \"error: attempting to allocate .* over limit\" err | wc -l) &&\n> +\ttest \"$n\" -gt 1\n> +'\n> +\n>  test_done\n"},{"id":"245089","messageId":"53ADBE51.9050005@virtuell-zuhause.de","threadId":"36765","inReplyTo":"xmqqegyb5zeh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2014-06-27T18:56:17Z","receivedAt":"2014-06-27T18:56:17Z","isPatch":true,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am 26.06.2014 19:55, schrieb Junio C Hamano:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n> \n>> Too large files may lead to failure to allocate memory. If it happens\n>> here, it could impact quite a few commands that involve\n>> diff. Moreover, too large files are inefficient to compare anyway (and\n>> most likely non-text), so mark them binary and skip looking at their\n>> content.\n>> ...\n>> diff --git a/diff.c b/diff.c\n>> index a489540..7a977aa 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n>>  \t\t\tone->is_binary = one->driver->binary;\n>>  \t\telse {\n>>  \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n>> -\t\t\t\tdiff_populate_filespec(one, 0);\n>> -\t\t\tif (one->data)\n>> +\t\t\t\tdiff_populate_filespec(one, DIFF_POPULATE_IS_BINARY);\n>> +\t\t\tif (one->is_binary == -1 && one->data)\n>>  \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n>>  \t\t\t\t\t\tone->size);\n>>  \t\t\tif (one->is_binary == -1)\n> \n> The name is misleading and forced me to read it twice before I\n> realized that this is \"populating the is-binary bit\".  It might make\n> it a bit better if you renamed it to DIFF_POPULATE_IS_BINARY_BIT or\n> perhaps DIFF_POPULATE_CHECK_BINARY or something.  For consistency,\n> the other bit may want to be also renamed from SIZE_ONLY to either\n> \n>  (1) CHECK_SIZE_ONLY\n> \n>  (2) One bit for CHECK_SIZE, another for NO_CONTENTS, and optionally\n>      make SIZE_ONLY the union of two\n> \n> I do not have strong preference either way; the latter may be more\n> logical in that \"not loading contents\" and \"check size\" are sort of\n> orthogonal in that you can later choose to check, without loading\n> contents, only the binary-ness without checking size, but no calles\n> that passes a non-zero flag to the populate-filespec function will\n> want to slurp in the contents in practice, so in that sense we could\n> declare that the NO_CONENTS bit is implied.\n> \n> But more importantly, would this patch actually help?  For one\n> thing, this wouldn't (and shouldn't) help if the user wants --binary\n> diff out of us anyway, I suspect, but I wonder what the following\n> codepath in the builtin_diff() function would do:\n> \n> \t\t...\n> \t} else if (!DIFF_OPT_TST(o, TEXT) &&\n> \t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n> \t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n> \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n> \t\t\tdie(\"unable to read files to diff\");\n> \t\t/* Quite common confusing case */\n> \t\tif (mf1.size == mf2.size &&\n> \t\t    !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {\n> \t\t\tif (must_show_header)\n> \t\t\t\tfprintf(o->file, \"%s\", header.buf);\n> \t\t\tgoto free_ab_and_return;\n> \t\t}\n> \t\tfprintf(o->file, \"%s\", header.buf);\n> \t\tstrbuf_reset(&header);\n> \t\tif (DIFF_OPT_TST(o, BINARY))\n> \t\t\temit_binary_diff(o->file, &mf1, &mf2, line_prefix);\n> \t\telse\n> \t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n> \t\t\t\tline_prefix, lbl[0], lbl[1]);\n> \t\to->found_changes = 1;\n> \t} else {\n> \t\t...\n> \n> If we weren't told with --text/-a to force textual output, and\n> at least one of the sides is marked as binary (and this patch marks\n> a large blob as binary unless attributes says otherwise), we still\n> call fill_mmfile() on them to slurp the contents to be compared, no?\n> \n> And before you get to the DIFF_OPT_TST(o, BINARY), you memcmp(3) to\n> check if the sides are identical, so...\n\nGood point. So how about an additional change roughly sketched as\n\n@@ -2223,6 +2223,14 @@ struct userdiff_driver *get_textconv(struct\ndiff_filespec *one)\n \treturn userdiff_get_textconv(one->driver);\n }\n\n+/* read the files in small chunks into memory and compare them */\n+static int filecmp_chunked(struct diff_filespec *one,\n+\tstruct diff_filespec *two)\n+{\n+\t// TODO add implementation\n+\treturn 0;\n+}\n+\n static void builtin_diff(const char *name_a,\n \t\t\t const char *name_b,\n \t\t\t struct diff_filespec *one,\n@@ -2325,19 +2333,26 @@ static void builtin_diff(const char *name_a,\n \t} else if (!DIFF_OPT_TST(o, TEXT) &&\n \t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n \t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n-\t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2,two)< 0)\n-\t\t\tdie(\"unable to read files to diff\");\n+\n+\t\tunsigned long size1 = diff_filespec_size(one);\n+\t\tunsigned long size2 = diff_filespec_size(two);\n+\n+\t\tif (size1 == 0 || size2 == 0)\n+\t\t\tdie(\"unable to retrieve file sizes for diff\");\n \t\t/* Quite common confusing case */\n-\t\tif (mf1.size == mf2.size &&\n-\t\t    !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {\n+\t\tif (size1 == size2 && !filecmp_chunked(one,two)) {\n \t\t\tif (must_show_header)\n \t\t\t\tfprintf(o->file, \"%s\", header.buf);\n \t\t\tgoto free_ab_and_return;\n \t\t}\n \t\tfprintf(o->file, \"%s\", header.buf);\n \t\tstrbuf_reset(&header);\n-\t\tif (DIFF_OPT_TST(o, BINARY))\n+\t\tif (DIFF_OPT_TST(o, BINARY)) {\n+\t\t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n+\t\t\t\tdie(\"unable to read files to diff\");\n \t\t\temit_binary_diff(o->file, &mf1, &mf2, line_prefix);\n+\t\t}\n \t\telse\n \t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n \t\t\t\tline_prefix, lbl[0], lbl[1]);\n\nThen the default diff case, no BINARY flag, would not read both files into memory.\nfilecmp_chunked will be slower than file_mmfile and memcmp, but its whole purpose is to read and compare the files in chunks.\nThe chunk size can be something like 64MiB.\n"},{"id":"245142","messageId":"CACsJy8C8TkQLNXdz=D8YC8TjSEP1nth8LmvftJYSG0bTRvqU7A@mail.gmail.com","threadId":"36765","inReplyTo":"xmqqa98z5yrg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/4] fsck: do not die when not enough memory to examine a pack entry","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-29T00:40:19Z","receivedAt":"2014-06-29T00:40:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jun 27, 2014 at 1:09 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> fsck is a tool that error() is more preferred than die(), but many\n>\n> \"more preferred\" without justifying why it is \"more preferred\" is\n> not quite a justification, is it?  Also, an object failing to load\n> in-core is not a missing object, so if your aim is to let \"fsck\"\n> diagnose a too-large-to-load object as missing and let it continue,\n> I do not know if it is \"more preferred\" in the first place.  Adding\n> a \"too large--cannot check\" bin of objects may be needed for it to\n> be useful.  Also, we might need to give at the end \"oh by the way,\n> because we couldn't read some objects to even determine its type,\n> the unreachable report from this fsck run is totally useless.\"\n\nFair enough. I think avoiding dying in xmalloc() in this code path is\nstill a good thing. At least \"failed to read object %s\" is more\ninformative than simply \"Out of memory\". The error cascading effect in\nfsck is something I think we already have. I'll try to rephrase the\ncommit message. But if you think this is not a good direction,\ndropping it is not so bad.\n\nI'm going to look at xmalloc() in unpack-objects. That's where we\nreally should not abort because of memory shortage as the user may try\nto get as many objects as possible out of the pack.\n\n> The log message tries to justify that this may be a good thing for\n> \"fsck\", but the patch actually tries to change the behaviour of all\n> code paths that try to load an object in-core without considering\n> the ramifications of such a change.  I _think_ all callers should be\n> prepared to receive NULL when we encounter a corrupt object (and\n> otherwise we should fix them), but it is unclear how much audit of\n> the callers (if any) was done to prepare this change.\n-- \nDuy\n"},{"id":"245143","messageId":"CACsJy8BobwPCnyBgYJw6yVZF_05aKeaBGRe=C4GxvUL4V58y_Q@mail.gmail.com","threadId":"36765","inReplyTo":"53ADBE51.9050005@virtuell-zuhause.de","subject":"Re: [PATCH v2 4/4] diff: mark any file larger than core.bigfilethreshold binary","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-29T01:11:16Z","receivedAt":"2014-06-29T01:11:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jun 28, 2014 at 1:56 AM, Thomas Braun\n<thomas.braun@virtuell-zuhause.de> wrote:\n>> The name is misleading and forced me to read it twice before I\n>> realized that this is \"populating the is-binary bit\".  It might make\n>> it a bit better if you renamed it to DIFF_POPULATE_IS_BINARY_BIT or\n>> perhaps DIFF_POPULATE_CHECK_BINARY or something.  For consistency,\n>> the other bit may want to be also renamed from SIZE_ONLY to either\n>>\n>>  (1) CHECK_SIZE_ONLY\n>>\n>>  (2) One bit for CHECK_SIZE, another for NO_CONTENTS, and optionally\n>>      make SIZE_ONLY the union of two\n>>\n>> I do not have strong preference either way; the latter may be more\n>> logical in that \"not loading contents\" and \"check size\" are sort of\n>> orthogonal in that you can later choose to check, without loading\n>> contents, only the binary-ness without checking size, but no calles\n>> that passes a non-zero flag to the populate-filespec function will\n>> want to slurp in the contents in practice, so in that sense we could\n>> declare that the NO_CONENTS bit is implied.\n\nWill do (and probably go with (1) as I still prefer zero as \"good defaults\")\n\n>> But more importantly, would this patch actually help?\n\nWell yes as demonstrated by the new test ;-) Unfortunately the scope\nof help is limited to --stat.. I should have done more thorough\ntesting.\n\n>> For one\n>> thing, this wouldn't (and shouldn't) help if the user wants --binary\n>> diff out of us anyway, I suspect, but I wonder what the following\n>> codepath in the builtin_diff() function would do:\n>>\n>>               ...\n>>       } else if (!DIFF_OPT_TST(o, TEXT) &&\n>>           ( (!textconv_one && diff_filespec_is_binary(one)) ||\n>>             (!textconv_two && diff_filespec_is_binary(two)) )) {\n>>               if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n>>                       die(\"unable to read files to diff\");\n>>               /* Quite common confusing case */\n>>               if (mf1.size == mf2.size &&\n>>                   !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {\n>>                       if (must_show_header)\n>>                               fprintf(o->file, \"%s\", header.buf);\n>>                       goto free_ab_and_return;\n>>               }\n>>               fprintf(o->file, \"%s\", header.buf);\n>>               strbuf_reset(&header);\n>>               if (DIFF_OPT_TST(o, BINARY))\n>>                       emit_binary_diff(o->file, &mf1, &mf2, line_prefix);\n>>               else\n>>                       fprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n>>                               line_prefix, lbl[0], lbl[1]);\n>>               o->found_changes = 1;\n>>       } else {\n>>               ...\n>>\n>> If we weren't told with --text/-a to force textual output, and\n>> at least one of the sides is marked as binary (and this patch marks\n>> a large blob as binary unless attributes says otherwise), we still\n>> call fill_mmfile() on them to slurp the contents to be compared, no?\n>>\n>> And before you get to the DIFF_OPT_TST(o, BINARY), you memcmp(3) to\n>> check if the sides are identical, so...\n>\n> Good point. So how about an additional change roughly sketched as\n>\n> @@ -2223,6 +2223,14 @@ struct userdiff_driver *get_textconv(struct\n> diff_filespec *one)\n>         return userdiff_get_textconv(one->driver);\n>  }\n>\n> +/* read the files in small chunks into memory and compare them */\n> +static int filecmp_chunked(struct diff_filespec *one,\n> +       struct diff_filespec *two)\n> +{\n> +       // TODO add implementation\n> +       return 0;\n> +}\n> +\n\n\nWe have object streaming interface to do similar like this. In fact\nindex-pack already does large file memcmp() for hash collision test. I\nthink I can move some code around and support large file in this code\npath..\n-- \nDuy\n"},{"id":"247656","messageId":"1407927454-9268-1-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1403610336-27761-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 0/6] Large file improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:28Z","receivedAt":"2014-08-13T10:57:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Since v2:\n\n - reword the fsck patch to center around unpack_compressed_entry instead\n - make sure unpack-objects survive out of memory error because of large files\n - rename diff_filespec_population flags\n - make \"git diff <tree> <tree>\" work on large files\n\nNguyễn Thái Ngọc Duy (6):\n  wrapper.c: introduce gentle xmalloc(z) that does not die()\n  sha1_file.c: do not die failing to malloc in unpack_compressed_entry\n  unpack-objects: continue when fail to malloc due to large objects\n  diff.c: allow to pass more flags to diff_populate_filespec\n  diff --stat: mark any file larger than core.bigfilethreshold binary\n  diff: shortcut for diff'ing two binary SHA-1 objects\n\n Documentation/config.txt        |  3 +-\n Documentation/gitattributes.txt |  4 +--\n builtin/unpack-objects.c        | 42 +++++++++++++++++++++++-\n diff.c                          | 52 +++++++++++++++++++++--------\n diffcore-rename.c               |  6 ++--\n diffcore.h                      |  4 ++-\n git-compat-util.h               |  2 ++\n sha1_file.c                     |  4 ++-\n t/t1050-large.sh                | 25 ++++++++++++++\n wrapper.c                       | 73 ++++++++++++++++++++++++++++++++---------\n 10 files changed, 177 insertions(+), 38 deletions(-)\n\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247650","messageId":"1407927454-9268-2-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 1/6] wrapper.c: introduce gentle xmalloc(z) that does not die()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:29Z","receivedAt":"2014-08-13T10:57:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n git-compat-util.h |  2 ++\n wrapper.c         | 73 +++++++++++++++++++++++++++++++++++++++++++------------\n 2 files changed, 59 insertions(+), 16 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..0e541e7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -592,7 +592,9 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);\n #endif\n extern char *xstrdup(const char *str);\n extern void *xmalloc(size_t size);\n+extern void *xmalloc_gentle(size_t size);\n extern void *xmallocz(size_t size);\n+extern void *xmallocz_gentle(size_t size);\n extern void *xmemdupz(const void *data, size_t len);\n extern char *xstrndup(const char *str, size_t len);\n extern void *xrealloc(void *ptr, size_t size);\ndiff --git a/wrapper.c b/wrapper.c\nindex bc1bfb8..ad0992a 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -9,16 +9,23 @@ static void do_nothing(size_t size)\n \n static void (*try_to_free_routine)(size_t size) = do_nothing;\n \n-static void memory_limit_check(size_t size)\n+static int memory_limit_check(size_t size, int gentle)\n {\n \tstatic int limit = -1;\n \tif (limit == -1) {\n \t\tconst char *env = getenv(\"GIT_ALLOC_LIMIT\");\n \t\tlimit = env ? atoi(env) * 1024 : 0;\n \t}\n-\tif (limit && size > limit)\n-\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n-\t\t    (intmax_t)size, limit);\n+\tif (limit && size > limit) {\n+\t\tif (gentle) {\n+\t\t\terror(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t      (intmax_t)size, limit);\n+\t\t\treturn -1;\n+\t\t} else\n+\t\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t    (intmax_t)size, limit);\n+\t}\n+\treturn 0;\n }\n \n try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n@@ -42,11 +49,12 @@ char *xstrdup(const char *str)\n \treturn ret;\n }\n \n-void *xmalloc(size_t size)\n+static void *do_xmalloc(size_t size, int gentle)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tif (memory_limit_check(size, gentle))\n+\t\treturn NULL;\n \tret = malloc(size);\n \tif (!ret && !size)\n \t\tret = malloc(1);\n@@ -55,9 +63,16 @@ void *xmalloc(size_t size)\n \t\tret = malloc(size);\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n-\t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t    (unsigned long)size);\n+\t\tif (!ret) {\n+\t\t\tif (!gentle)\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t    (unsigned long)size);\n+\t\t\telse {\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t      (unsigned long)size);\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t}\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n@@ -65,16 +80,42 @@ void *xmalloc(size_t size)\n \treturn ret;\n }\n \n-void *xmallocz(size_t size)\n+void *xmalloc(size_t size)\n+{\n+\treturn do_xmalloc(size, 0);\n+}\n+\n+void *xmalloc_gentle(size_t size)\n+{\n+\treturn do_xmalloc(size, 1);\n+}\n+\n+static void *do_xmallocz(size_t size, int gentle)\n {\n \tvoid *ret;\n-\tif (unsigned_add_overflows(size, 1))\n-\t\tdie(\"Data too large to fit into virtual memory space.\");\n-\tret = xmalloc(size + 1);\n-\t((char*)ret)[size] = 0;\n+\tif (unsigned_add_overflows(size, 1)) {\n+\t\tif (gentle) {\n+\t\t\terror(\"Data too large to fit into virtual memory space.\");\n+\t\t\treturn NULL;\n+\t\t} else\n+\t\t\tdie(\"Data too large to fit into virtual memory space.\");\n+\t}\n+\tret = do_xmalloc(size + 1, gentle);\n+\tif (ret)\n+\t\t((char*)ret)[size] = 0;\n \treturn ret;\n }\n \n+void *xmallocz(size_t size)\n+{\n+\treturn do_xmallocz(size, 0);\n+}\n+\n+void *xmallocz_gentle(size_t size)\n+{\n+\treturn do_xmallocz(size, 1);\n+}\n+\n /*\n  * xmemdupz() allocates (len + 1) bytes of memory, duplicates \"len\" bytes of\n  * \"data\" to the allocated memory, zero terminates the allocated memory,\n@@ -96,7 +137,7 @@ void *xrealloc(void *ptr, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tmemory_limit_check(size, 0);\n \tret = realloc(ptr, size);\n \tif (!ret && !size)\n \t\tret = realloc(ptr, 1);\n@@ -115,7 +156,7 @@ void *xcalloc(size_t nmemb, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size * nmemb);\n+\tmemory_limit_check(size * nmemb, 0);\n \tret = calloc(nmemb, size);\n \tif (!ret && (!nmemb || !size))\n \t\tret = calloc(1, 1);\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247651","messageId":"1407927454-9268-3-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 2/6] sha1_file.c: do not die failing to malloc in unpack_compressed_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:30Z","receivedAt":"2014-08-13T10:57:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Fewer die() gives better control to the caller, provided that the\ncaller _can_ handle it. And in unpack_compressed_entry() case, it can,\nbecause unpack_compressed_entry() already returns NULL if it fails to\ninflate data.\n\nA side effect from this is fsck continues to run when very large blobs\nare present (and do not fit in memory).\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c      | 4 +++-\n t/t1050-large.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3f70b1d..330862b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1923,7 +1923,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n \tgit_zstream stream;\n \tunsigned char *buffer, *in;\n \n-\tbuffer = xmallocz(size);\n+\tbuffer = xmallocz_gentle(size);\n+\tif (!buffer)\n+\t\treturn NULL;\n \tmemset(&stream, 0, sizeof(stream));\n \tstream.next_out = buffer;\n \tstream.avail_out = size + 1;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex aea4936..5642f84 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '\n \tgit archive --format=zip HEAD >/dev/null\n '\n \n+test_expect_success 'fsck' '\n+\ttest_must_fail git fsck 2>err &&\n+\tn=$(grep \"error: attempting to allocate .* over limit\" err | wc -l) &&\n+\ttest \"$n\" -gt 1\n+'\n+\n test_done\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247652","messageId":"1407927454-9268-4-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 3/6] unpack-objects: continue when fail to malloc due to large objects","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:31Z","receivedAt":"2014-08-13T10:57:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"As a recovery tool, unpack-objects should go on unpacking as many\nobjects as it can.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/unpack-objects.c | 42 +++++++++++++++++++++++++++++++++++++++++-\n t/t1050-large.sh         |  7 +++++++\n 2 files changed, 48 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 99cde45..8b5c67e 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -88,10 +88,50 @@ static void use(int bytes)\n \tconsumed_bytes += bytes;\n }\n \n+static void inflate_and_throw_away(unsigned long size)\n+{\n+\tgit_zstream stream;\n+\tchar buf[8192];\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\tstream.next_out = (unsigned char *)buf;\n+\tstream.avail_out = sizeof(buf);\n+\tstream.next_in = fill(1);\n+\tstream.avail_in = len;\n+\tgit_inflate_init(&stream);\n+\n+\tfor (;;) {\n+\t\tint ret = git_inflate(&stream, 0);\n+\t\tuse(len - stream.avail_in);\n+\t\tif (stream.total_out == size && ret == Z_STREAM_END)\n+\t\t\tbreak;\n+\t\tif (ret != Z_OK) {\n+\t\t\terror(\"inflate returned %d\", ret);\n+\t\t\tif (!recover)\n+\t\t\t\texit(1);\n+\t\t\thas_errors = 1;\n+\t\t\tbreak;\n+\t\t}\n+\t\tstream.next_out = (unsigned char *)buf;\n+\t\tstream.avail_out = sizeof(buf);\n+\t\tstream.next_in = fill(1);\n+\t\tstream.avail_in = len;\n+\t}\n+\tgit_inflate_end(&stream);\n+}\n+\n static void *get_data(unsigned long size)\n {\n \tgit_zstream stream;\n-\tvoid *buf = xmalloc(size);\n+\tvoid *buf = xmalloc_gentle(size);\n+\n+\tif (!buf) {\n+\t\tif (!recover)\n+\t\t\texit(1);\n+\t\thas_errors = 1;\n+\t\tinflate_and_throw_away(size);\n+\t\treturn NULL;\n+\t}\n \n \tmemset(&stream, 0, sizeof(stream));\n \ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 5642f84..eec2cca 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -169,4 +169,11 @@ test_expect_success 'fsck' '\n \ttest \"$n\" -gt 1\n '\n \n+test_expect_success 'unpack-objects' '\n+\tP=`ls .git/objects/pack/*.pack` &&\n+\tgit unpack-objects -n -r <$P 2>err\n+\ttest $? = 1 &&\n+\tgrep \"error: attempting to allocate .* over limit\" err\n+'\n+\n test_done\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247653","messageId":"1407927454-9268-5-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 4/6] diff.c: allow to pass more flags to diff_populate_filespec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:32Z","receivedAt":"2014-08-13T10:57:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c            | 13 +++++++------\n diffcore-rename.c |  6 ++++--\n diffcore.h        |  3 ++-\n 3 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 867f034..f4b7421 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -376,7 +376,7 @@ static unsigned long diff_filespec_size(struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one))\n \t\treturn 0;\n-\tdiff_populate_filespec(one, 1);\n+\tdiff_populate_filespec(one, CHECK_SIZE_ONLY);\n \treturn one->size;\n }\n \n@@ -1910,11 +1910,11 @@ static void show_dirstat(struct diff_options *options)\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t\tdiff_free_filespec_data(p->two);\n \t\t} else if (DIFF_FILE_VALID(p->one)) {\n-\t\t\tdiff_populate_filespec(p->one, 1);\n+\t\t\tdiff_populate_filespec(p->one, CHECK_SIZE_ONLY);\n \t\t\tcopied = added = 0;\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t} else if (DIFF_FILE_VALID(p->two)) {\n-\t\t\tdiff_populate_filespec(p->two, 1);\n+\t\t\tdiff_populate_filespec(p->two, CHECK_SIZE_ONLY);\n \t\t\tcopied = 0;\n \t\t\tadded = p->two->size;\n \t\t\tdiff_free_filespec_data(p->two);\n@@ -2668,8 +2668,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n  * grab the data for the blob (or file) for our own in-core comparison.\n  * diff_filespec has data and size fields for this purpose.\n  */\n-int diff_populate_filespec(struct diff_filespec *s, int size_only)\n+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n {\n+\tint size_only = flags & CHECK_SIZE_ONLY;\n \tint err = 0;\n \t/*\n \t * demote FAIL to WARN to allow inspecting the situation\n@@ -4688,8 +4689,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)\n \t    !DIFF_FILE_VALID(p->two) ||\n \t    (p->one->sha1_valid && p->two->sha1_valid) ||\n \t    (p->one->mode != p->two->mode) ||\n-\t    diff_populate_filespec(p->one, 1) ||\n-\t    diff_populate_filespec(p->two, 1) ||\n+\t    diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||\n+\t    diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||\n \t    (p->one->size != p->two->size) ||\n \t    !diff_filespec_is_identical(p->one, p->two)) /* (2) */\n \t\tp->skip_stat_unmatch_result = 1;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 2e44a37..4e132f1 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,\n \t * is a possible size - we really should have a flag to\n \t * say whether the size is valid or not!)\n \t */\n-\tif (!src->cnt_data && diff_populate_filespec(src, 1))\n+\tif (!src->cnt_data &&\n+\t    diff_populate_filespec(src, CHECK_SIZE_ONLY))\n \t\treturn 0;\n-\tif (!dst->cnt_data && diff_populate_filespec(dst, 1))\n+\tif (!dst->cnt_data &&\n+\t    diff_populate_filespec(dst, CHECK_SIZE_ONLY))\n \t\treturn 0;\n \n \tmax_size = ((src->size > dst->size) ? src->size : dst->size);\ndiff --git a/diffcore.h b/diffcore.h\nindex c876dac..c80df18 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);\n extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n-extern int diff_populate_filespec(struct diff_filespec *, int);\n+#define CHECK_SIZE_ONLY 1\n+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\n extern int diff_filespec_is_binary(struct diff_filespec *);\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247655","messageId":"1407927454-9268-6-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 5/6] diff --stat: mark any file larger than core.bigfilethreshold binary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:33Z","receivedAt":"2014-08-13T10:57:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Too large files may lead to failure to allocate memory. If it happens\nhere, it could impact quite a few commands that involve\ndiff. Moreover, too large files are inefficient to compare anyway (and\nmost likely non-text), so mark them binary and skip looking at their\ncontent.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt        |  3 ++-\n Documentation/gitattributes.txt |  4 ++--\n diff.c                          | 26 ++++++++++++++++++--------\n diffcore.h                      |  1 +\n t/t1050-large.sh                |  4 ++++\n 5 files changed, 27 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c55c22a..53df40e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -499,7 +499,8 @@ core.bigFileThreshold::\n \tFiles larger than this size are stored deflated, without\n \tattempting delta compression.  Storing large files without\n \tdelta compression avoids excessive memory usage, at the\n-\tslight expense of increased disk usage.\n+\tslight expense of increased disk usage. Additionally files\n+\tlarger than this size are allways treated as binary.\n +\n Default is 512 MiB on all platforms.  This should be reasonable\n for most projects as source code and other text files can still\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 643c1ba..9b45bda 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -440,8 +440,8 @@ Unspecified::\n \n \tA path to which the `diff` attribute is unspecified\n \tfirst gets its contents inspected, and if it looks like\n-\ttext, it is treated as text.  Otherwise it would\n-\tgenerate `Binary files differ`.\n+\ttext and is smaller than core.bigFileThreshold, it is treated\n+\tas text. Otherwise it would generate `Binary files differ`.\n \n String::\n \ndiff --git a/diff.c b/diff.c\nindex f4b7421..d381a6f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2188,8 +2188,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \t\t\tone->is_binary = one->driver->binary;\n \t\telse {\n \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\t\t\tdiff_populate_filespec(one, 0);\n-\t\t\tif (one->data)\n+\t\t\t\tdiff_populate_filespec(one, CHECK_BINARY);\n+\t\t\tif (one->is_binary == -1 && one->data)\n \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n \t\t\t\t\t\tone->size);\n \t\t\tif (one->is_binary == -1)\n@@ -2725,6 +2725,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n+\t\tif ((flags & CHECK_BINARY) &&\n+\t\t    s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\ts->is_binary = 1;\n+\t\t\treturn 0;\n+\t\t}\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n@@ -2746,16 +2751,21 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t}\n \telse {\n \t\tenum object_type type;\n-\t\tif (size_only) {\n+\t\tif (size_only || (flags & CHECK_BINARY)) {\n \t\t\ttype = sha1_object_info(s->sha1, &s->size);\n \t\t\tif (type < 0)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t} else {\n-\t\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n-\t\t\tif (!s->data)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t\ts->should_free = 1;\n+\t\t\tif (size_only)\n+\t\t\t\treturn 0;\n+\t\t\tif (s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\t\ts->is_binary = 1;\n+\t\t\t\treturn 0;\n+\t\t\t}\n \t\t}\n+\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n+\t\tif (!s->data)\n+\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n+\t\ts->should_free = 1;\n \t}\n \treturn 0;\n }\ndiff --git a/diffcore.h b/diffcore.h\nindex c80df18..33ea2de 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n #define CHECK_SIZE_ONLY 1\n+#define CHECK_BINARY    2\n extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex eec2cca..711f22c 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '\n \tgit diff --raw HEAD^\n '\n \n+test_expect_success 'diff --stat' '\n+\tgit diff --stat HEAD^ HEAD\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247654","messageId":"1407927454-9268-7-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 6/6] diff: shortcut for diff'ing two binary SHA-1 objects","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-13T10:57:34Z","receivedAt":"2014-08-13T10:57:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If we are given two SHA-1 and asked to determine if they are different\n(but not _what_ differences), we know right away by comparing SHA-1.\n\nA side effect of this patch is, because large files are marked binary,\ndiff-tree will not need to unpack them. 'diff-index --cached' will not\neither. But 'diff-files' still does.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c           | 13 +++++++++++++\n t/t1050-large.sh |  8 ++++++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex d381a6f..b85bcfb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2324,6 +2324,19 @@ static void builtin_diff(const char *name_a,\n \t} else if (!DIFF_OPT_TST(o, TEXT) &&\n \t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n \t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n+\t\tif (!one->data && !two->data &&\n+\t\t    S_ISREG(one->mode) && S_ISREG(two->mode) &&\n+\t\t    !DIFF_OPT_TST(o, BINARY)) {\n+\t\t\tif (!hashcmp(one->sha1, two->sha1)) {\n+\t\t\t\tif (must_show_header)\n+\t\t\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\t\tgoto free_ab_and_return;\n+\t\t\t}\n+\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n+\t\t\t\tline_prefix, lbl[0], lbl[1]);\n+\t\t\tgoto free_ab_and_return;\n+\t\t}\n \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\t\tdie(\"unable to read files to diff\");\n \t\t/* Quite common confusing case */\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 711f22c..b294963 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -116,6 +116,14 @@ test_expect_success 'diff --stat' '\n \tgit diff --stat HEAD^ HEAD\n '\n \n+test_expect_success 'diff' '\n+\tgit diff HEAD^ HEAD\n+'\n+\n+test_expect_success 'diff --cached' '\n+\tgit diff --cached HEAD^\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247683","messageId":"CAPig+cRwe0+RjLRpCmvFGczmy5x6J7Kz43XsMiOOL2trVQn4bw@mail.gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 5/6] diff --stat: mark any file larger than core.bigfilethreshold binary","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-08-13T19:32:14Z","receivedAt":"2014-08-13T19:32:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Aug 13, 2014 at 6:57 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> Too large files may lead to failure to allocate memory. If it happens\n> here, it could impact quite a few commands that involve\n> diff. Moreover, too large files are inefficient to compare anyway (and\n> most likely non-text), so mark them binary and skip looking at their\n> content.\n>\n> Noticed-by: Dale R. Worley <worley@alum.mit.edu>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  Documentation/config.txt        |  3 ++-\n>  Documentation/gitattributes.txt |  4 ++--\n>  diff.c                          | 26 ++++++++++++++++++--------\n>  diffcore.h                      |  1 +\n>  t/t1050-large.sh                |  4 ++++\n>  5 files changed, 27 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index c55c22a..53df40e 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -499,7 +499,8 @@ core.bigFileThreshold::\n>         Files larger than this size are stored deflated, without\n>         attempting delta compression.  Storing large files without\n>         delta compression avoids excessive memory usage, at the\n> -       slight expense of increased disk usage.\n> +       slight expense of increased disk usage. Additionally files\n> +       larger than this size are allways treated as binary.\n\ns/allways/always/\n\n>  Default is 512 MiB on all platforms.  This should be reasonable\n>  for most projects as source code and other text files can still\n"},{"id":"247708","messageId":"CAPc5daX3WNsSQ1epor+K7T8ePjhthGdTivecaD9GeeDnQRJCAg@mail.gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 2/6] sha1_file.c: do not die failing to malloc in unpack_compressed_entry","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-13T21:13:39Z","receivedAt":"2014-08-13T21:13:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Looks very sensible. Thanks.\n\nOn Wed, Aug 13, 2014 at 3:57 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> Fewer die() gives better control to the caller, provided that the\n> caller _can_ handle it. And in unpack_compressed_entry() case, it can,\n> because unpack_compressed_entry() already returns NULL if it fails to\n> inflate data.\n>\n> A side effect from this is fsck continues to run when very large blobs\n> are present (and do not fit in memory).\n>\n> Noticed-by: Dale R. Worley <worley@alum.mit.edu>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  sha1_file.c      | 4 +++-\n>  t/t1050-large.sh | 6 ++++++\n>  2 files changed, 9 insertions(+), 1 deletion(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 3f70b1d..330862b 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1923,7 +1923,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n>         git_zstream stream;\n>         unsigned char *buffer, *in;\n>\n> -       buffer = xmallocz(size);\n> +       buffer = xmallocz_gentle(size);\n> +       if (!buffer)\n> +               return NULL;\n>         memset(&stream, 0, sizeof(stream));\n>         stream.next_out = buffer;\n>         stream.avail_out = size + 1;\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index aea4936..5642f84 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '\n>         git archive --format=zip HEAD >/dev/null\n>  '\n>\n> +test_expect_success 'fsck' '\n> +       test_must_fail git fsck 2>err &&\n> +       n=$(grep \"error: attempting to allocate .* over limit\" err | wc -l) &&\n> +       test \"$n\" -gt 1\n> +'\n> +\n>  test_done\n> --\n> 2.1.0.rc0.78.gc0d8480\n>\n"},{"id":"247720","messageId":"xmqqegwj2fhr.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1407927454-9268-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 1/6] wrapper.c: introduce gentle xmalloc(z) that does not die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-14T16:38:40Z","receivedAt":"2014-08-14T16:38:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n\nI think the basic idea is sound.\n\n\"git grep -e _gentle -e _gently -e _gentler\" hints me that the new\nfunctions are somewhat misnamed, though.\n\n>  git-compat-util.h |  2 ++\n>  wrapper.c         | 73 +++++++++++++++++++++++++++++++++++++++++++------------\n>  2 files changed, 59 insertions(+), 16 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index f587749..0e541e7 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -592,7 +592,9 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);\n>  #endif\n>  extern char *xstrdup(const char *str);\n>  extern void *xmalloc(size_t size);\n> +extern void *xmalloc_gentle(size_t size);\n>  extern void *xmallocz(size_t size);\n> +extern void *xmallocz_gentle(size_t size);\n>  extern void *xmemdupz(const void *data, size_t len);\n>  extern char *xstrndup(const char *str, size_t len);\n>  extern void *xrealloc(void *ptr, size_t size);\n> diff --git a/wrapper.c b/wrapper.c\n> index bc1bfb8..ad0992a 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -9,16 +9,23 @@ static void do_nothing(size_t size)\n>  \n>  static void (*try_to_free_routine)(size_t size) = do_nothing;\n>  \n> -static void memory_limit_check(size_t size)\n> +static int memory_limit_check(size_t size, int gentle)\n>  {\n>  \tstatic int limit = -1;\n>  \tif (limit == -1) {\n>  \t\tconst char *env = getenv(\"GIT_ALLOC_LIMIT\");\n>  \t\tlimit = env ? atoi(env) * 1024 : 0;\n>  \t}\n> -\tif (limit && size > limit)\n> -\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n> -\t\t    (intmax_t)size, limit);\n> +\tif (limit && size > limit) {\n> +\t\tif (gentle) {\n> +\t\t\terror(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n> +\t\t\t      (intmax_t)size, limit);\n> +\t\t\treturn -1;\n> +\t\t} else\n> +\t\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n> +\t\t\t    (intmax_t)size, limit);\n> +\t}\n> +\treturn 0;\n>  }\n>  \n>  try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n> @@ -42,11 +49,12 @@ char *xstrdup(const char *str)\n>  \treturn ret;\n>  }\n>  \n> -void *xmalloc(size_t size)\n> +static void *do_xmalloc(size_t size, int gentle)\n>  {\n>  \tvoid *ret;\n>  \n> -\tmemory_limit_check(size);\n> +\tif (memory_limit_check(size, gentle))\n> +\t\treturn NULL;\n>  \tret = malloc(size);\n>  \tif (!ret && !size)\n>  \t\tret = malloc(1);\n> @@ -55,9 +63,16 @@ void *xmalloc(size_t size)\n>  \t\tret = malloc(size);\n>  \t\tif (!ret && !size)\n>  \t\t\tret = malloc(1);\n> -\t\tif (!ret)\n> -\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> -\t\t\t    (unsigned long)size);\n> +\t\tif (!ret) {\n> +\t\t\tif (!gentle)\n> +\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> +\t\t\t\t    (unsigned long)size);\n> +\t\t\telse {\n> +\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> +\t\t\t\t      (unsigned long)size);\n> +\t\t\t\treturn NULL;\n> +\t\t\t}\n> +\t\t}\n>  \t}\n>  #ifdef XMALLOC_POISON\n>  \tmemset(ret, 0xA5, size);\n> @@ -65,16 +80,42 @@ void *xmalloc(size_t size)\n>  \treturn ret;\n>  }\n>  \n> -void *xmallocz(size_t size)\n> +void *xmalloc(size_t size)\n> +{\n> +\treturn do_xmalloc(size, 0);\n> +}\n> +\n> +void *xmalloc_gentle(size_t size)\n> +{\n> +\treturn do_xmalloc(size, 1);\n> +}\n> +\n> +static void *do_xmallocz(size_t size, int gentle)\n>  {\n>  \tvoid *ret;\n> -\tif (unsigned_add_overflows(size, 1))\n> -\t\tdie(\"Data too large to fit into virtual memory space.\");\n> -\tret = xmalloc(size + 1);\n> -\t((char*)ret)[size] = 0;\n> +\tif (unsigned_add_overflows(size, 1)) {\n> +\t\tif (gentle) {\n> +\t\t\terror(\"Data too large to fit into virtual memory space.\");\n> +\t\t\treturn NULL;\n> +\t\t} else\n> +\t\t\tdie(\"Data too large to fit into virtual memory space.\");\n> +\t}\n> +\tret = do_xmalloc(size + 1, gentle);\n> +\tif (ret)\n> +\t\t((char*)ret)[size] = 0;\n>  \treturn ret;\n>  }\n>  \n> +void *xmallocz(size_t size)\n> +{\n> +\treturn do_xmallocz(size, 0);\n> +}\n> +\n> +void *xmallocz_gentle(size_t size)\n> +{\n> +\treturn do_xmallocz(size, 1);\n> +}\n> +\n>  /*\n>   * xmemdupz() allocates (len + 1) bytes of memory, duplicates \"len\" bytes of\n>   * \"data\" to the allocated memory, zero terminates the allocated memory,\n> @@ -96,7 +137,7 @@ void *xrealloc(void *ptr, size_t size)\n>  {\n>  \tvoid *ret;\n>  \n> -\tmemory_limit_check(size);\n> +\tmemory_limit_check(size, 0);\n>  \tret = realloc(ptr, size);\n>  \tif (!ret && !size)\n>  \t\tret = realloc(ptr, 1);\n> @@ -115,7 +156,7 @@ void *xcalloc(size_t nmemb, size_t size)\n>  {\n>  \tvoid *ret;\n>  \n> -\tmemory_limit_check(size * nmemb);\n> +\tmemory_limit_check(size * nmemb, 0);\n>  \tret = calloc(nmemb, size);\n>  \tif (!ret && (!nmemb || !size))\n>  \t\tret = calloc(1, 1);\n"},{"id":"247721","messageId":"xmqq7g2b2ele.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1407927454-9268-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 3/6] unpack-objects: continue when fail to malloc due to large objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-14T16:58:05Z","receivedAt":"2014-08-14T16:58:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> As a recovery tool, unpack-objects should go on unpacking as many\n> objects as it can.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  builtin/unpack-objects.c | 42 +++++++++++++++++++++++++++++++++++++++++-\n>  t/t1050-large.sh         |  7 +++++++\n>  2 files changed, 48 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\n> index 99cde45..8b5c67e 100644\n> --- a/builtin/unpack-objects.c\n> +++ b/builtin/unpack-objects.c\n> @@ -88,10 +88,50 @@ static void use(int bytes)\n>  \tconsumed_bytes += bytes;\n>  }\n>  \n> +static void inflate_and_throw_away(unsigned long size)\n> +{\n> +\tgit_zstream stream;\n> +\tchar buf[8192];\n> +\n> +\tmemset(&stream, 0, sizeof(stream));\n> +\tstream.next_out = (unsigned char *)buf;\n> +\tstream.avail_out = sizeof(buf);\n> +\tstream.next_in = fill(1);\n> +\tstream.avail_in = len;\n> +\tgit_inflate_init(&stream);\n> +\n> +\tfor (;;) {\n> +\t\tint ret = git_inflate(&stream, 0);\n> +\t\tuse(len - stream.avail_in);\n> +\t\tif (stream.total_out == size && ret == Z_STREAM_END)\n> +\t\t\tbreak;\n> +\t\tif (ret != Z_OK) {\n> +\t\t\terror(\"inflate returned %d\", ret);\n> +\t\t\tif (!recover)\n> +\t\t\t\texit(1);\n> +\t\t\thas_errors = 1;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tstream.next_out = (unsigned char *)buf;\n> +\t\tstream.avail_out = sizeof(buf);\n> +\t\tstream.next_in = fill(1);\n> +\t\tstream.avail_in = len;\n> +\t}\n> +\tgit_inflate_end(&stream);\n> +}\n\nThis looks wrong in a few ways.\n\nYou already know that we saw an error when you get to this function,\nwhether you will see an out-of-sync stream in this loop or not, so\nthere is no reason to copy the assignment to has_errors from the\nother function.  You also know 'recover' is true---otherwise you\nwouldn't be here.\n\nBut more importantly, the basic structure of this loop is the same\nas the loop we already have in the only caller of this new function,\nnot just the regular \"zlib produced this much that is not yet the\nexpected size, go on reading more\" and \"we are at the end of the\nstream with Z_STREAM_END, and we are done\", but even to \"the stream\nis corrupt, we need to exit the loop\", they are identical.  Is a\ncopy-and-paste like this the best we can do to add this \"skip to the\nend of the current stream\"?  We would really want to keep the number\nof copies of this loop down; we saw a same bug introduced on the\ntermination condition multiple times to different copies X-<.\n\n>  static void *get_data(unsigned long size)\n>  {\n>  \tgit_zstream stream;\n> -\tvoid *buf = xmalloc(size);\n> +\tvoid *buf = xmalloc_gentle(size);\n> +\n> +\tif (!buf) {\n> +\t\tif (!recover)\n> +\t\t\texit(1);\n> +\t\thas_errors = 1;\n> +\t\tinflate_and_throw_away(size);\n> +\t\treturn NULL;\n> +\t}\n>  \n>  \tmemset(&stream, 0, sizeof(stream));\n>  \n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index 5642f84..eec2cca 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -169,4 +169,11 @@ test_expect_success 'fsck' '\n>  \ttest \"$n\" -gt 1\n>  '\n>  \n> +test_expect_success 'unpack-objects' '\n> +\tP=`ls .git/objects/pack/*.pack` &&\n> +\tgit unpack-objects -n -r <$P 2>err\n> +\ttest $? = 1 &&\n> +\tgrep \"error: attempting to allocate .* over limit\" err\n> +'\n> +\n>  test_done\n"},{"id":"247722","messageId":"xmqq38cz2ehr.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1407927454-9268-7-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 6/6] diff: shortcut for diff'ing two binary SHA-1 objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-14T17:00:16Z","receivedAt":"2014-08-14T17:00:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index 711f22c..b294963 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -116,6 +116,14 @@ test_expect_success 'diff --stat' '\n>  \tgit diff --stat HEAD^ HEAD\n>  '\n>  \n> +test_expect_success 'diff' '\n> +\tgit diff HEAD^ HEAD\n> +'\n> +\n> +test_expect_success 'diff --cached' '\n> +\tgit diff --cached HEAD^\n> +'\n\nWhat are these checking?  No check for their outcome?\n\n>  test_expect_success 'hash-object' '\n>  \tgit hash-object large1\n>  '\n"},{"id":"247723","messageId":"xmqqy4ur0z46.fsf@gitster.dls.corp.google.com","threadId":"36765","inReplyTo":"1407927454-9268-7-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 6/6] diff: shortcut for diff'ing two binary SHA-1 objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-14T17:17:45Z","receivedAt":"2014-08-14T17:17:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> If we are given two SHA-1 and asked to determine if they are different\n> (but not _what_ differences), we know right away by comparing SHA-1.\n>\n> A side effect of this patch is, because large files are marked binary,\n> diff-tree will not need to unpack them. 'diff-index --cached' will not\n> either. But 'diff-files' still does.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  diff.c           | 13 +++++++++++++\n>  t/t1050-large.sh |  8 ++++++++\n>  2 files changed, 21 insertions(+)\n>\n> diff --git a/diff.c b/diff.c\n> index d381a6f..b85bcfb 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2324,6 +2324,19 @@ static void builtin_diff(const char *name_a,\n>  \t} else if (!DIFF_OPT_TST(o, TEXT) &&\n>  \t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n>  \t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n> +\t\tif (!one->data && !two->data &&\n> +\t\t    S_ISREG(one->mode) && S_ISREG(two->mode) &&\n> +\t\t    !DIFF_OPT_TST(o, BINARY)) {\n> +\t\t\tif (!hashcmp(one->sha1, two->sha1)) {\n> +\t\t\t\tif (must_show_header)\n> +\t\t\t\t\tfprintf(o->file, \"%s\", header.buf);\n> +\t\t\t\tgoto free_ab_and_return;\n> +\t\t\t}\n> +\t\t\tfprintf(o->file, \"%s\", header.buf);\n> +\t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n> +\t\t\t\tline_prefix, lbl[0], lbl[1]);\n> +\t\t\tgoto free_ab_and_return;\n> +\t\t}\n\nA tangent.\n\nI think one and two can point at the same object only when this\nfilepair is involved in rename/copy.  In other words, one and two\nwith the same <mode,sha1,name> would not be given to this code.  And\nmust-show-header would be set to true long before we get here in\nfill-metainfo in such a case.\n\nI think this new code and the original below which you copied this\none from can probably be simplified.  It already felt wrong to see\ntwo copies of \"fprintf(o->file \"%s\", header.buf)\" and now we have\nfour of them.  Because this is a copy-and-paste of the identical\nlogic from below, I do not want you to attempt fixing this tangent\nin this patch, though.\n\nThanks.\n\n>  \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n>  \t\t\tdie(\"unable to read files to diff\");\n>  \t\t/* Quite common confusing case */\n> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n> index 711f22c..b294963 100755\n> --- a/t/t1050-large.sh\n> +++ b/t/t1050-large.sh\n> @@ -116,6 +116,14 @@ test_expect_success 'diff --stat' '\n>  \tgit diff --stat HEAD^ HEAD\n>  '\n>  \n> +test_expect_success 'diff' '\n> +\tgit diff HEAD^ HEAD\n> +'\n> +\n> +test_expect_success 'diff --cached' '\n> +\tgit diff --cached HEAD^\n> +'\n> +\n>  test_expect_success 'hash-object' '\n>  \tgit hash-object large1\n>  '\n"},{"id":"247733","messageId":"CACsJy8AF-NdAL-4t9k2-qrQCFFRC8T6==SvYiP14FktrxHEj=w@mail.gmail.com","threadId":"36765","inReplyTo":"xmqq7g2b2ele.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 3/6] unpack-objects: continue when fail to malloc due to large objects","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-15T05:24:54Z","receivedAt":"2014-08-15T05:24:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Aug 14, 2014 at 11:58 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> +static void inflate_and_throw_away(unsigned long size)\n>> +{\n>\n> But more importantly, the basic structure of this loop is the same\n> as the loop we already have in the only caller of this new function,\n> not just the regular \"zlib produced this much that is not yet the\n> expected size, go on reading more\" and \"we are at the end of the\n> stream with Z_STREAM_END, and we are done\", but even to \"the stream\n> is corrupt, we need to exit the loop\", they are identical.  Is a\n> copy-and-paste like this the best we can do to add this \"skip to the\n> end of the current stream\"?  We would really want to keep the number\n> of copies of this loop down; we saw a same bug introduced on the\n> termination condition multiple times to different copies X-<.\n\nI know. I'm the author of one of those bugs. I considered updating the\ninflate loop in get_data() to support this throw-away mode but was\nafraid I may make the same mistake like in index-pack again. I'll\nprobably drop this patch and think if I can unify inflate loop (for\nother places too, not just unpack-objects).\n-- \nDuy\n"},{"id":"247740","messageId":"CACsJy8CO-5+Nodh19kiWP-YEFChqptf7_nFs=zFvzNJ23HmOgQ@mail.gmail.com","threadId":"36765","inReplyTo":"xmqq38cz2ehr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 6/6] diff: shortcut for diff'ing two binary SHA-1 objects","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-15T12:11:26Z","receivedAt":"2014-08-15T12:11:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Aug 15, 2014 at 12:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> diff --git a/t/t1050-large.sh b/t/t1050-large.sh\n>> index 711f22c..b294963 100755\n>> --- a/t/t1050-large.sh\n>> +++ b/t/t1050-large.sh\n>> @@ -116,6 +116,14 @@ test_expect_success 'diff --stat' '\n>>       git diff --stat HEAD^ HEAD\n>>  '\n>>\n>> +test_expect_success 'diff' '\n>> +     git diff HEAD^ HEAD\n>> +'\n>> +\n>> +test_expect_success 'diff --cached' '\n>> +     git diff --cached HEAD^\n>> +'\n>\n> What are these checking?  No check for their outcome?\n\nThe first test in this file set $GIT_ALLOC_LIMIT and these commands\nwould not succeed without the patch. But yes I can check the \"binary\nfiles differ\" too.\n-- \nDuy\n"},{"id":"247765","messageId":"1408158486-7328-1-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1407927454-9268-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 0/5] Large file improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:01Z","receivedAt":"2014-08-16T03:08:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Since v3:\n\n - rename xmallocz_gentle to xmallocz_gently\n - drop the unpack-objects patch\n - fix allways type\n\nNguyễn Thái Ngọc Duy (5):\n  wrapper.c: introduce gentle xmallocz that does not die()\n  sha1_file.c: do not die failing to malloc in unpack_compressed_entry\n  diff.c: allow to pass more flags to diff_populate_filespec\n  diff --stat: mark any file larger than core.bigfilethreshold binary\n  diff: shortcut for diff'ing two binary SHA-1 objects\n\n Documentation/config.txt        |  3 +-\n Documentation/gitattributes.txt |  4 +--\n diff.c                          | 52 ++++++++++++++++++++++---------\n diffcore-rename.c               |  6 ++--\n diffcore.h                      |  4 ++-\n git-compat-util.h               |  1 +\n sha1_file.c                     |  4 ++-\n t/t1050-large.sh                | 20 ++++++++++++\n wrapper.c                       | 68 +++++++++++++++++++++++++++++++----------\n 9 files changed, 125 insertions(+), 37 deletions(-)\n\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247767","messageId":"1408158486-7328-2-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1408158486-7328-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 1/5] wrapper.c: introduce gentle xmallocz that does not die()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:02Z","receivedAt":"2014-08-16T03:08:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n git-compat-util.h |  1 +\n wrapper.c         | 68 ++++++++++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 53 insertions(+), 16 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..8785fd3 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -593,6 +593,7 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);\n extern char *xstrdup(const char *str);\n extern void *xmalloc(size_t size);\n extern void *xmallocz(size_t size);\n+extern void *xmallocz_gently(size_t size);\n extern void *xmemdupz(const void *data, size_t len);\n extern char *xstrndup(const char *str, size_t len);\n extern void *xrealloc(void *ptr, size_t size);\ndiff --git a/wrapper.c b/wrapper.c\nindex bc1bfb8..dc9c8f4 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -9,16 +9,23 @@ static void do_nothing(size_t size)\n \n static void (*try_to_free_routine)(size_t size) = do_nothing;\n \n-static void memory_limit_check(size_t size)\n+static int memory_limit_check(size_t size, int gentle)\n {\n \tstatic int limit = -1;\n \tif (limit == -1) {\n \t\tconst char *env = getenv(\"GIT_ALLOC_LIMIT\");\n \t\tlimit = env ? atoi(env) * 1024 : 0;\n \t}\n-\tif (limit && size > limit)\n-\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n-\t\t    (intmax_t)size, limit);\n+\tif (limit && size > limit) {\n+\t\tif (gentle) {\n+\t\t\terror(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t      (intmax_t)size, limit);\n+\t\t\treturn -1;\n+\t\t} else\n+\t\t\tdie(\"attempting to allocate %\"PRIuMAX\" over limit %d\",\n+\t\t\t    (intmax_t)size, limit);\n+\t}\n+\treturn 0;\n }\n \n try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n@@ -42,11 +49,12 @@ char *xstrdup(const char *str)\n \treturn ret;\n }\n \n-void *xmalloc(size_t size)\n+static void *do_xmalloc(size_t size, int gentle)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tif (memory_limit_check(size, gentle))\n+\t\treturn NULL;\n \tret = malloc(size);\n \tif (!ret && !size)\n \t\tret = malloc(1);\n@@ -55,9 +63,16 @@ void *xmalloc(size_t size)\n \t\tret = malloc(size);\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n-\t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t    (unsigned long)size);\n+\t\tif (!ret) {\n+\t\t\tif (!gentle)\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t    (unsigned long)size);\n+\t\t\telse {\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t\t      (unsigned long)size);\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t}\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n@@ -65,16 +80,37 @@ void *xmalloc(size_t size)\n \treturn ret;\n }\n \n-void *xmallocz(size_t size)\n+void *xmalloc(size_t size)\n+{\n+\treturn do_xmalloc(size, 0);\n+}\n+\n+static void *do_xmallocz(size_t size, int gentle)\n {\n \tvoid *ret;\n-\tif (unsigned_add_overflows(size, 1))\n-\t\tdie(\"Data too large to fit into virtual memory space.\");\n-\tret = xmalloc(size + 1);\n-\t((char*)ret)[size] = 0;\n+\tif (unsigned_add_overflows(size, 1)) {\n+\t\tif (gentle) {\n+\t\t\terror(\"Data too large to fit into virtual memory space.\");\n+\t\t\treturn NULL;\n+\t\t} else\n+\t\t\tdie(\"Data too large to fit into virtual memory space.\");\n+\t}\n+\tret = do_xmalloc(size + 1, gentle);\n+\tif (ret)\n+\t\t((char*)ret)[size] = 0;\n \treturn ret;\n }\n \n+void *xmallocz(size_t size)\n+{\n+\treturn do_xmallocz(size, 0);\n+}\n+\n+void *xmallocz_gently(size_t size)\n+{\n+\treturn do_xmallocz(size, 1);\n+}\n+\n /*\n  * xmemdupz() allocates (len + 1) bytes of memory, duplicates \"len\" bytes of\n  * \"data\" to the allocated memory, zero terminates the allocated memory,\n@@ -96,7 +132,7 @@ void *xrealloc(void *ptr, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size);\n+\tmemory_limit_check(size, 0);\n \tret = realloc(ptr, size);\n \tif (!ret && !size)\n \t\tret = realloc(ptr, 1);\n@@ -115,7 +151,7 @@ void *xcalloc(size_t nmemb, size_t size)\n {\n \tvoid *ret;\n \n-\tmemory_limit_check(size * nmemb);\n+\tmemory_limit_check(size * nmemb, 0);\n \tret = calloc(nmemb, size);\n \tif (!ret && (!nmemb || !size))\n \t\tret = calloc(1, 1);\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247766","messageId":"1408158486-7328-3-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1408158486-7328-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 2/5] sha1_file.c: do not die failing to malloc in unpack_compressed_entry","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:03Z","receivedAt":"2014-08-16T03:08:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Fewer die() gives better control to the caller, provided that the\ncaller _can_ handle it. And in unpack_compressed_entry() case, it can,\nbecause unpack_compressed_entry() already returns NULL if it fails to\ninflate data.\n\nA side effect from this is fsck continues to run when very large blobs\nare present (and do not fit in memory).\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n sha1_file.c      | 4 +++-\n t/t1050-large.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3f70b1d..8db73f0 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1923,7 +1923,9 @@ static void *unpack_compressed_entry(struct packed_git *p,\n \tgit_zstream stream;\n \tunsigned char *buffer, *in;\n \n-\tbuffer = xmallocz(size);\n+\tbuffer = xmallocz_gently(size);\n+\tif (!buffer)\n+\t\treturn NULL;\n \tmemset(&stream, 0, sizeof(stream));\n \tstream.next_out = buffer;\n \tstream.avail_out = size + 1;\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex aea4936..5642f84 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '\n \tgit archive --format=zip HEAD >/dev/null\n '\n \n+test_expect_success 'fsck' '\n+\ttest_must_fail git fsck 2>err &&\n+\tn=$(grep \"error: attempting to allocate .* over limit\" err | wc -l) &&\n+\ttest \"$n\" -gt 1\n+'\n+\n test_done\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247768","messageId":"1408158486-7328-4-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1408158486-7328-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 3/5] diff.c: allow to pass more flags to diff_populate_filespec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:04Z","receivedAt":"2014-08-16T03:08:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c            | 13 +++++++------\n diffcore-rename.c |  6 ++++--\n diffcore.h        |  3 ++-\n 3 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 867f034..f4b7421 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -376,7 +376,7 @@ static unsigned long diff_filespec_size(struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one))\n \t\treturn 0;\n-\tdiff_populate_filespec(one, 1);\n+\tdiff_populate_filespec(one, CHECK_SIZE_ONLY);\n \treturn one->size;\n }\n \n@@ -1910,11 +1910,11 @@ static void show_dirstat(struct diff_options *options)\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t\tdiff_free_filespec_data(p->two);\n \t\t} else if (DIFF_FILE_VALID(p->one)) {\n-\t\t\tdiff_populate_filespec(p->one, 1);\n+\t\t\tdiff_populate_filespec(p->one, CHECK_SIZE_ONLY);\n \t\t\tcopied = added = 0;\n \t\t\tdiff_free_filespec_data(p->one);\n \t\t} else if (DIFF_FILE_VALID(p->two)) {\n-\t\t\tdiff_populate_filespec(p->two, 1);\n+\t\t\tdiff_populate_filespec(p->two, CHECK_SIZE_ONLY);\n \t\t\tcopied = 0;\n \t\t\tadded = p->two->size;\n \t\t\tdiff_free_filespec_data(p->two);\n@@ -2668,8 +2668,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n  * grab the data for the blob (or file) for our own in-core comparison.\n  * diff_filespec has data and size fields for this purpose.\n  */\n-int diff_populate_filespec(struct diff_filespec *s, int size_only)\n+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n {\n+\tint size_only = flags & CHECK_SIZE_ONLY;\n \tint err = 0;\n \t/*\n \t * demote FAIL to WARN to allow inspecting the situation\n@@ -4688,8 +4689,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)\n \t    !DIFF_FILE_VALID(p->two) ||\n \t    (p->one->sha1_valid && p->two->sha1_valid) ||\n \t    (p->one->mode != p->two->mode) ||\n-\t    diff_populate_filespec(p->one, 1) ||\n-\t    diff_populate_filespec(p->two, 1) ||\n+\t    diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||\n+\t    diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||\n \t    (p->one->size != p->two->size) ||\n \t    !diff_filespec_is_identical(p->one, p->two)) /* (2) */\n \t\tp->skip_stat_unmatch_result = 1;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 2e44a37..4e132f1 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,\n \t * is a possible size - we really should have a flag to\n \t * say whether the size is valid or not!)\n \t */\n-\tif (!src->cnt_data && diff_populate_filespec(src, 1))\n+\tif (!src->cnt_data &&\n+\t    diff_populate_filespec(src, CHECK_SIZE_ONLY))\n \t\treturn 0;\n-\tif (!dst->cnt_data && diff_populate_filespec(dst, 1))\n+\tif (!dst->cnt_data &&\n+\t    diff_populate_filespec(dst, CHECK_SIZE_ONLY))\n \t\treturn 0;\n \n \tmax_size = ((src->size > dst->size) ? src->size : dst->size);\ndiff --git a/diffcore.h b/diffcore.h\nindex c876dac..c80df18 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);\n extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n-extern int diff_populate_filespec(struct diff_filespec *, int);\n+#define CHECK_SIZE_ONLY 1\n+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\n extern int diff_filespec_is_binary(struct diff_filespec *);\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247769","messageId":"1408158486-7328-5-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1408158486-7328-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 4/5] diff --stat: mark any file larger than core.bigfilethreshold binary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:05Z","receivedAt":"2014-08-16T03:08:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Too large files may lead to failure to allocate memory. If it happens\nhere, it could impact quite a few commands that involve\ndiff. Moreover, too large files are inefficient to compare anyway (and\nmost likely non-text), so mark them binary and skip looking at their\ncontent.\n\nNoticed-by: Dale R. Worley <worley@alum.mit.edu>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt        |  3 ++-\n Documentation/gitattributes.txt |  4 ++--\n diff.c                          | 26 ++++++++++++++++++--------\n diffcore.h                      |  1 +\n t/t1050-large.sh                |  4 ++++\n 5 files changed, 27 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c55c22a..3b5b24a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -499,7 +499,8 @@ core.bigFileThreshold::\n \tFiles larger than this size are stored deflated, without\n \tattempting delta compression.  Storing large files without\n \tdelta compression avoids excessive memory usage, at the\n-\tslight expense of increased disk usage.\n+\tslight expense of increased disk usage. Additionally files\n+\tlarger than this size are always treated as binary.\n +\n Default is 512 MiB on all platforms.  This should be reasonable\n for most projects as source code and other text files can still\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 643c1ba..9b45bda 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -440,8 +440,8 @@ Unspecified::\n \n \tA path to which the `diff` attribute is unspecified\n \tfirst gets its contents inspected, and if it looks like\n-\ttext, it is treated as text.  Otherwise it would\n-\tgenerate `Binary files differ`.\n+\ttext and is smaller than core.bigFileThreshold, it is treated\n+\tas text. Otherwise it would generate `Binary files differ`.\n \n String::\n \ndiff --git a/diff.c b/diff.c\nindex f4b7421..d381a6f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2188,8 +2188,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)\n \t\t\tone->is_binary = one->driver->binary;\n \t\telse {\n \t\t\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\t\t\tdiff_populate_filespec(one, 0);\n-\t\t\tif (one->data)\n+\t\t\t\tdiff_populate_filespec(one, CHECK_BINARY);\n+\t\t\tif (one->is_binary == -1 && one->data)\n \t\t\t\tone->is_binary = buffer_is_binary(one->data,\n \t\t\t\t\t\tone->size);\n \t\t\tif (one->is_binary == -1)\n@@ -2725,6 +2725,11 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n+\t\tif ((flags & CHECK_BINARY) &&\n+\t\t    s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\ts->is_binary = 1;\n+\t\t\treturn 0;\n+\t\t}\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n@@ -2746,16 +2751,21 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t}\n \telse {\n \t\tenum object_type type;\n-\t\tif (size_only) {\n+\t\tif (size_only || (flags & CHECK_BINARY)) {\n \t\t\ttype = sha1_object_info(s->sha1, &s->size);\n \t\t\tif (type < 0)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t} else {\n-\t\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n-\t\t\tif (!s->data)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n-\t\t\ts->should_free = 1;\n+\t\t\tif (size_only)\n+\t\t\t\treturn 0;\n+\t\t\tif (s->size > big_file_threshold && s->is_binary == -1) {\n+\t\t\t\ts->is_binary = 1;\n+\t\t\t\treturn 0;\n+\t\t\t}\n \t\t}\n+\t\ts->data = read_sha1_file(s->sha1, &type, &s->size);\n+\t\tif (!s->data)\n+\t\t\tdie(\"unable to read %s\", sha1_to_hex(s->sha1));\n+\t\ts->should_free = 1;\n \t}\n \treturn 0;\n }\ndiff --git a/diffcore.h b/diffcore.h\nindex c80df18..33ea2de 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,\n \t\t\t  int, unsigned short);\n \n #define CHECK_SIZE_ONLY 1\n+#define CHECK_BINARY    2\n extern int diff_populate_filespec(struct diff_filespec *, unsigned int);\n extern void diff_free_filespec_data(struct diff_filespec *);\n extern void diff_free_filespec_blob(struct diff_filespec *);\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 5642f84..00d2f33 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '\n \tgit diff --raw HEAD^\n '\n \n+test_expect_success 'diff --stat' '\n+\tgit diff --stat HEAD^ HEAD\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"247770","messageId":"1408158486-7328-6-git-send-email-pclouds@gmail.com","threadId":"36765","inReplyTo":"1408158486-7328-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 5/5] diff: shortcut for diff'ing two binary SHA-1 objects","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-16T03:08:06Z","receivedAt":"2014-08-16T03:08:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If we are given two SHA-1 and asked to determine if they are different\n(but not _what_ differences), we know right away by comparing SHA-1.\n\nA side effect of this patch is, because large files are marked binary,\ndiff-tree will not need to unpack them. 'diff-index --cached' will not\neither. But 'diff-files' still does.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c           | 13 +++++++++++++\n t/t1050-large.sh | 10 ++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex d381a6f..b85bcfb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2324,6 +2324,19 @@ static void builtin_diff(const char *name_a,\n \t} else if (!DIFF_OPT_TST(o, TEXT) &&\n \t    ( (!textconv_one && diff_filespec_is_binary(one)) ||\n \t      (!textconv_two && diff_filespec_is_binary(two)) )) {\n+\t\tif (!one->data && !two->data &&\n+\t\t    S_ISREG(one->mode) && S_ISREG(two->mode) &&\n+\t\t    !DIFF_OPT_TST(o, BINARY)) {\n+\t\t\tif (!hashcmp(one->sha1, two->sha1)) {\n+\t\t\t\tif (must_show_header)\n+\t\t\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\t\tgoto free_ab_and_return;\n+\t\t\t}\n+\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n+\t\t\t\tline_prefix, lbl[0], lbl[1]);\n+\t\t\tgoto free_ab_and_return;\n+\t\t}\n \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\t\tdie(\"unable to read files to diff\");\n \t\t/* Quite common confusing case */\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 00d2f33..05a1e1d 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -116,6 +116,16 @@ test_expect_success 'diff --stat' '\n \tgit diff --stat HEAD^ HEAD\n '\n \n+test_expect_success 'diff' '\n+\tgit diff HEAD^ HEAD >actual &&\n+\tgrep \"Binary files.*differ\" actual\n+'\n+\n+test_expect_success 'diff --cached' '\n+\tgit diff --cached HEAD^ >actual &&\n+\tgrep \"Binary files.*differ\" actual\n+'\n+\n test_expect_success 'hash-object' '\n \tgit hash-object large1\n '\n-- \n2.1.0.rc0.78.gc0d8480\n"}]}