Samuel Bronson naes...@gmail.com writes:
The -O flag really shouldn't silently fail to do anything when given a
path that it can't read from.
However, it should be able to read from un-mappable files, such as
pipes/fifos, /dev/null (as we document in the next patch), or in fact
*any* empty file (since Linux 2.6.12).
Could you enlighten the commit log readers a bit better here? Those
who know the change in 2.6.12 (i.e. 'mmapping with length 0 must
fail', says SUSv3, so we fail) you have in mind would know what you
mean by in fact any empty file even if you did not have (since
Linux 2.6.12), but those who do not know it would not be helped
with just (since Linux 2.6.12).
(Especially since we will be
documenting -O/dev/null to override diff.orderfile when we add that.)
(Note: -O/dev/null did have the right effect, since the existing error
handling essentially worked out to silently ignore the orderfile.)
So lets toss all of that logic to get the file mmapped and just use
strbuf_read_file() instead, which gives us decent error handling
practically for free.
Sounds good. In the longer term, we may want to move this
file-scope static to per-infocation struct diff_options and clean
up the storage used to hold the list of path patterns after we are
done with the diff, but that is outside the scope of this series.
Thanks.
Signed-off-by: Samuel Bronson naes...@gmail.com
---
diffcore-order.c | 23 ---
t/t4056-diff-order.sh | 23 +++
2 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/diffcore-order.c b/diffcore-order.c
index 23e9385..a63f332 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -10,28 +10,21 @@ static int order_cnt;
static void prepare_order(const char *orderfile)
{
- int fd, cnt, pass;
+ int cnt, pass;
+ struct strbuf sb = STRBUF_INIT;
void *map;
char *cp, *endp;
- struct stat st;
- size_t sz;
+ ssize_t sz;
if (order)
return;
- fd = open(orderfile, O_RDONLY);
- if (fd 0)
- return;
- if (fstat(fd, st)) {
- close(fd);
- return;
- }
- sz = xsize_t(st.st_size);
- map = mmap(NULL, sz, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
- close(fd);
- if (map == MAP_FAILED)
- return;
+ sz = strbuf_read_file(sb, orderfile, 0);
+ if (sz 0)
+ die_errno(_(failed to read orderfile '%s'), orderfile);
+ map = strbuf_detach(sb, NULL);
endp = (char *) map + sz;
+
for (pass = 0; pass 2; pass++) {
cnt = 0;
cp = map;
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index 398b3f6..eb471e7 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -61,12 +61,35 @@ test_expect_success no order (=tree object order) '
test_cmp expect_none actual
'
+test_expect_success 'missing orderfile' '
+ rm -f bogus_file
+ test_must_fail git diff -Obogus_file --name-only HEAD^..HEAD
+'
+
+test_expect_success 'unreadable orderfile' '
+ touch unreadable_file
+ chmod -r unreadable_file
+ test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD
+'
+
+test_expect_success 'orderfile is a directory' '
+ test_must_fail git diff -O/ --name-only HEAD^..HEAD
+'
+
for i in 1 2
do
test_expect_success orderfile using option ($i) '
git diff -Oorder_file_$i --name-only HEAD^..HEAD actual
test_cmp expect_$i actual
'
+
+ test_expect_success PIPE orderfile is fifo ($i) '
+ rm -f order_fifo
+ mkfifo order_fifo
+ cat order_file_$i order_fifo
+ git diff -O order_fifo --name-only HEAD^..HEAD actual
+ test_cmp expect_$i actual
+'
done
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
