- 14 9月, 2017 3 次提交
-
-
由 Jeff King 提交于
The return value of write_in_full() is either "-1", or the requested number of bytes[1]. If we make a partial write before seeing an error, we still return -1, not a partial value. This goes back to f6aa66cb (write_in_full: really write in full or return error on disk full., 2007-01-11). So checking anything except "was the return value negative" is pointless. And there are a couple of reasons not to do so: 1. It can do a funny signed/unsigned comparison. If your "len" is signed (e.g., a size_t) then the compiler will promote the "-1" to its unsigned variant. This works out for "!= len" (unless you really were trying to write the maximum size_t bytes), but is a bug if you check "< len" (an example of which was fixed recently in config.c). We should avoid promoting the mental model that you need to check the length at all, so that new sites are not tempted to copy us. 2. Checking for a negative value is shorter to type, especially when the length is an expression. 3. Linus says so. In d34cf19b (Clean up write_in_full() users, 2007-01-11), right after the write_in_full() semantics were changed, he wrote: I really wish every "write_in_full()" user would just check against "<0" now, but this fixes the nasty and stupid ones. Appeals to authority aside, this makes it clear that writing it this way does not have an intentional benefit. It's a historical curiosity that we never bothered to clean up (and which was undoubtedly cargo-culted into new sites). So let's convert these obviously-correct cases (this includes write_str_in_full(), which is just a wrapper for write_in_full()). [1] A careful reader may notice there is one way that write_in_full() can return a different value. If we ask write() to write N bytes and get a return value that is _larger_ than N, we could return a larger total. But besides the fact that this would imply a totally broken version of write(), it would already invoke undefined behavior. Our internal remaining counter is an unsigned size_t, which means that subtracting too many byte will wrap it around to a very large number. So we'll instantly begin reading off the end of the buffer, trying to write gigabytes (or petabytes) of data. Signed-off-by: NJeff King <peff@peff.net> Reviewed-by: NJonathan Nieder <jrnieder@gmail.com> Signed-off-by: NJunio C Hamano <gitster@pobox.com>
-
由 Jeff King 提交于
We ask to write 41 bytes and make sure that the return value is at least 41. This is the same "dangerous" pattern that was fixed in the prior commit (wherein a negative return value is promoted to unsigned), though it is not dangerous here because our "41" is a constant, not an unsigned variable. But we should convert it anyway to avoid modeling a dangerous construct. Signed-off-by: NJeff King <peff@peff.net> Signed-off-by: NJunio C Hamano <gitster@pobox.com>
-
由 Jeff King 提交于
The return type of write_in_full() is a signed ssize_t, because we may return "-1" on failure (even if we succeeded in writing some bytes). But "len" itself is may be an unsigned type (the function takes a size_t, but of course we may have something else in the calling function). So while it seems like: if (write_in_full(fd, buf, len) < len) die_errno("write error"); would trigger on error, it won't if "len" is unsigned. The compiler sees a signed/unsigned comparison and promotes the signed value, resulting in (size_t)-1, the highest possible size_t (or again, whatever type the caller has). This cannot possibly be smaller than "len", and so the conditional can never trigger. I scoured the code base for cases of this, but it turns out that these two in git_config_set_multivar_in_file_gently() are the only ones. Here our "len" is the difference between two size_t variables, making the result an unsigned size_t. We can fix this by just checking for a negative return value directly, as write_in_full() will never return any value except -1 or the full count. There's no addition to the test suite here, since you need to convince write() to fail in order to see the problem. The simplest reproduction recipe I came up with is to trigger ENOSPC: # make a limited-size filesystem dd if=/dev/zero of=small.disk bs=1M count=1 mke2fs small.disk mkdir mnt sudo mount -o loop small.disk mnt cd mnt sudo chown $USER:$USER . # make a config file with some content git config --file=config one.key value git config --file=config two.key value # now fill up the disk dd if=/dev/zero of=fill # and try to delete a key, which requires copying the rest # of the file to config.lock, and will fail on write() git config --file=config --unset two.key That final command should (and does after this patch) produce an error message due to the failed write, and leave the file intact. Instead, it silently ignores the failure and renames config.lock into place, leaving you with a totally empty config file! Reported-by: Ndemerphq <demerphq@gmail.com> Signed-off-by: NJeff King <peff@peff.net> Reviewed-by: NJonathan Nieder <jrnieder@gmail.com> Signed-off-by: NJunio C Hamano <gitster@pobox.com>
-
- 10 9月, 2017 37 次提交
-
-
由 Junio C Hamano 提交于
Signed-off-by: NJunio C Hamano <gitster@pobox.com>
-
由 Junio C Hamano 提交于
Doc updates. * jt/doc-pack-objects-fix: Doc: clarify that pack-objects makes packs, plural
-
由 Junio C Hamano 提交于
Code clean-up. * jn/vcs-svn-cleanup: vcs-svn: move remaining repo_tree functions to fast_export.h vcs-svn: remove repo_delete wrapper function vcs-svn: remove custom mode constants vcs-svn: remove more unused prototypes and declarations
-
由 Junio C Hamano 提交于
Code clean-up. * bc/vcs-svn-cleanup: vcs-svn: rename repo functions to "svn_repo" vcs-svn: remove unused prototypes
-
由 Junio C Hamano 提交于
Doc clean-up. * jk/doc-the-this: doc: fix typo in sendemail.identity
-
由 Junio C Hamano 提交于
Code clean-up. * rs/commit-h-single-parent-cleanup: commit: remove unused inline function single_parent()
-
由 Junio C Hamano 提交于
Doc fix. * mg/format-ref-doc-fix: Documentation/git-for-each-ref: clarify peeling of tags for --format Documentation: use proper wording for ref format strings
-
由 Junio C Hamano 提交于
Code clean-up. * sb/submodule-parallel-update: submodule.sh: remove unused variable
-
由 Junio C Hamano 提交于
Test fix. * hv/t5526-andand-chain-fix: t5526: fix some broken && chains
-
由 Junio C Hamano 提交于
Code clean-up. * sb/sha1-file-cleanup: sha1_file: make read_info_alternates static
-
由 Junio C Hamano 提交于
Test simplification. * rs/t1002-do-not-use-sum: t1002: stop using sum(1)
-
由 Junio C Hamano 提交于
Doc update. * ah/doc-empty-string-is-false: doc: clarify "config --bool" behaviour with empty string
-
由 Junio C Hamano 提交于
Code clean-up. * rs/merge-microcleanup: merge: use skip_prefix()
-
由 Junio C Hamano 提交于
Code clean-up. * rs/find-pack-entry-bisection: sha1_file: avoid comparison if no packed hash matches the first byte
-
由 Junio C Hamano 提交于
Code clean-up. * rs/apply-lose-prefix-length: apply: remove prefix_length member from apply_state
-
由 Junio C Hamano 提交于
Message fix. * rj/add-chmod-error-message: builtin/add: add detail to a 'cannot chmod' error message
-
由 Junio C Hamano 提交于
Code clean-up. * jk/hashcmp-memcmp: hashcmp: use memcmp instead of open-coded loop
-
由 Junio C Hamano 提交于
A test fix. * rs/t3700-clean-leftover: t3700: fix broken test under !POSIXPERM
-
由 Junio C Hamano 提交于
A comment fix. * jc/perl-git-comment-typofix: perl/Git.pm: typofix in a comment
-
由 Junio C Hamano 提交于
Code clean-up. * mf/no-dashed-subcommands: scripts: use "git foo" not "git-foo"
-
由 Junio C Hamano 提交于
A test fix. * ab/ref-filter-no-contains: tests: don't give unportable ">" to "test" built-in, use -gt
-
由 Junio C Hamano 提交于
"git archive" did not work well with pathspecs and the export-ignore attribute. We may want to resurrect the "we don't archive an empty directory" bonus patch, but I do not mind merging the above early to 'next' and leave it as a separate follow-up enhancement. cf. <20170820090629.tumvqwzkromcykjf@sigill.intra.peff.net> * rs/archive-excluded-directory: archive: don't queue excluded directories archive: factor out helper functions for handling attributes t5001: add tests for export-ignore attributes and exclude pathspecs
-
由 Junio C Hamano 提交于
Killing "git merge --edit" before the editor returns control left the repository in a state with MERGE_MSG but without MERGE_HEAD, which incorrectly tells the subsequent "git commit" that there was a squash merge in progress. This has been fixed. * mg/killed-merge: merge: save merge state earlier merge: split write_merge_state in two merge: clarify call chain Documentation/git-merge: explain --continue
-
由 Junio C Hamano 提交于
"git apply" that is used as a better "patch -p1" failed to apply a taken from a file with CRLF line endings to a file with CRLF line endings. The root cause was because it misused convert_to_git() that tried to do "safe-crlf" processing by looking at the index entry at the same path, which is a nonsense---in that mode, "apply" is not working on the data in (or derived from) the index at all. This has been fixed. * tb/apply-with-crlf: apply: file commited with CRLF should roundtrip diff and apply convert: add SAFE_CRLF_KEEP_CRLF
-
由 Junio C Hamano 提交于
When handshake with a subprocess filter notices that the process asked for an unknown capability, Git did not report what program the offending subprocess was running. This has been corrected. We may want a follow-up fix to tighten the error checking, though. * cc/subprocess-handshake-missing-capabilities: sub-process: print the cmd when a capability is unsupported
-
由 Junio C Hamano 提交于
"git grep -L" and "git grep --quiet -L" reported different exit codes; this has been corrected. * as/grep-quiet-no-match-exit-code-fix: git-grep: correct exit code with --quiet and -L
-
由 Junio C Hamano 提交于
bash 4.4 or newer gave a warning on NUL byte in command substitution done in "git stash"; this has been squelched. * kd/stash-with-bash-4.4: stash: prevent warning about null bytes in input
-
由 Junio C Hamano 提交于
Memory leak in an error codepath has been plugged. * rs/win32-syslog-leakfix: win32: plug memory leak on realloc() failure in syslog()
-
由 Junio C Hamano 提交于
Memory leak in an error codepath has been plugged. * rs/unpack-entry-leakfix: sha1_file: release delta_stack on error in unpack_entry()
-
由 Junio C Hamano 提交于
Memory leak in an error codepath has been plugged. * rs/fsck-obj-leakfix: fsck: free buffers on error in fsck_obj()
-
由 Junio C Hamano 提交于
"git svn" used with "--localtime" option did not compute the tz offset for the timestamp in question and instead always used the current time, which has been corrected. * ur/svn-local-zone: git svn fetch: Create correct commit timestamp when using --localtime
-
由 Junio C Hamano 提交于
"git am -s" has been taught that some input may end with a trailer block that is not Signed-off-by: and it should refrain from adding an extra blank line before adding a new sign-off in such a case. * pw/am-signoff: am: fix signoff when other trailers are present
-
由 Junio C Hamano 提交于
Portability fix. * rs/in-obsd-basename-dirname-take-const: test-path-utils: handle const parameter of basename and dirname
-
由 Junio C Hamano 提交于
Test portability fix. * rs/t4062-obsd: t4062: use less than 256 repetitions in regex
-
由 Junio C Hamano 提交于
Test portability fix for BSDs. * rs/obsd-getcwd-workaround: t0001: skip test with restrictive permissions if getpwd(3) respects them
-
由 Junio C Hamano 提交于
"git clone --recurse-submodules --quiet" did not pass the quiet option down to submodules. * bw/clone-recursive-quiet: clone: teach recursive clones to respect -q
-
由 Junio C Hamano 提交于
Commands like "git rebase" accepted the --rerere-autoupdate option from the command line, but did not always use it. This has been fixed. * pw/sequence-rerere-autoupdate: cherry-pick/revert: reject --rerere-autoupdate when continuing cherry-pick/revert: remember --rerere-autoupdate t3504: use test_commit rebase -i: honor --rerere-autoupdate rebase: honor --rerere-autoupdate am: remember --rerere-autoupdate setting
-